Skip to content

Support old syntax for DataType parsing - #8541

Merged
alamb merged 4 commits into
apache:mainfrom
alamb:alamb/fix_datatype_parsing
Oct 3, 2025
Merged

alamb merged 4 commits into
apache:mainfrom
alamb:alamb/fix_datatype_parsing

Conversation

@alamb

@alamb alamb commented Oct 2, 2025 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Systems like DataFusion use the string representation of DataType in their public APIs but the type names have changed after

We should retain backwards compatibility with the old type names too

What changes are included in this PR?

  1. Support old style Timestamp(Nanosecond, None) and Timestamp(Nanosecond, None) style timestamp specifiers

Are these changes tested?

Yes, with new tests

Are there any user-facing changes?

See above

@github-actions github-actions Bot added parquet Changes to the parquet crate arrow Changes to the arrow crate parquet-variant parquet-variant* crates labels Oct 2, 2025
@alamb
alamb force-pushed the alamb/fix_datatype_parsing branch from ce539c6 to 64abb2a Compare October 2, 2025 16:25
@github-actions github-actions Bot removed parquet Changes to the parquet crate parquet-variant parquet-variant* crates labels Oct 2, 2025
@alamb
alamb marked this pull request as ready for review October 2, 2025 17:13
@mbrobbel mbrobbel added this to the 57.0.0 milestone Oct 2, 2025

@mbrobbel mbrobbel 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.

Looks good, thanks.

@emilk emilk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! Should we document that any changes to datatype_parse should be backwards compatible?

@alamb

alamb commented Oct 3, 2025

Copy link
Copy Markdown
Contributor Author

Thanks! Should we document that any changes to datatype_parse should be backwards compatible?

Yes, thank you for that suggestion

I added comments and additional tests for backwards compatibility

r#"Timestamp(Second, Some("+00:00"))"#,
Timestamp(Second, Some("+00:00".into())),
),
("Null", Null),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

these tests now encode all the current round trip cases in an explicit backwards compatiblity mode test

@alamb
alamb merged commit 9d03e41 into apache:main Oct 3, 2025
26 checks passed
@alamb
alamb deleted the alamb/fix_datatype_parsing branch October 3, 2025 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DataType parsing no longer works correctly for old formatted timestamps

4 participants