Skip to content

Introduce invalid data error macro - #2928

Open
xanderbailey wants to merge 1 commit into
apache:mainfrom
xanderbailey:xb/invalid_data_macro
Open

Introduce invalid data error macro#2928
xanderbailey wants to merge 1 commit into
apache:mainfrom
xanderbailey:xb/invalid_data_macro

Conversation

@xanderbailey

@xanderbailey xanderbailey commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Today we construct DataInvalid like so: which ends up being very verbose in my opinion

Error::new(
    ErrorKind::DataInvalid,
    "Partition column is not a StructArray",
)
Screenshot 2026-07-30 at 15 15 15

What changes are included in this PR?

Introduce invalid_data! macro as a shorthand for this error.

The error above becomes:

invalid_data!("Partition column is not a StructArray")

Are these changes tested?

AI Disclosure

Claude wrote the script to do this migration but I have reviewed manually.

@xanderbailey
xanderbailey force-pushed the xb/invalid_data_macro branch from 7a4473f to b4c444d Compare July 30, 2026 14:28
@xanderbailey
xanderbailey marked this pull request as ready for review July 30, 2026 14:29
@xanderbailey

Copy link
Copy Markdown
Contributor Author

Not sure what folks think about this change but I personally think it improves readability.

@anoopj anoopj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup!

/// // Attaching a source error
/// let n: i32 = s.parse().map_err(|e| invalid_data!("not an int: {s}").with_source(e))?;
/// ```
macro_rules! invalid_data {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one design thought: the expr arm accepts any expression, so if somone writes invalid_data!(format!(...)) (out of habit), we will double allocate. Not worth guarding against though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants