Skip to content

Conversation

@etseidl
Copy link
Contributor

@etseidl etseidl commented Nov 19, 2025

Which issue does this PR close?

Rationale for this change

See issue...a prior PR changed the behavior of parsing lists. This PR temporarily reverts to the old behavior, but retains the ability to parse List(nullable Int64).

What changes are included in this PR?

Always sets nullable to true regardless of the presence or absence of the nullable token.

Are these changes tested?

Yes

Are there any user-facing changes?

No, this reverts to previous behavior.

@github-actions github-actions bot added the arrow Changes to the arrow crate label Nov 19, 2025
@etseidl etseidl requested a review from alamb November 19, 2025 22:54
fixed size lists fixed in a different PR so don't workaround for them
@alamb
Copy link
Contributor

alamb commented Nov 20, 2025

Thanks @etseidl

The other thing I was thinking about was potentially changing the display so to List(Int64) means nullable (the default) and List(nonnull Int64) or something means non nullable (a more strict requirement).

Since we haven't released List(nullable ..) syntax yet I think we can change it

@etseidl
Copy link
Contributor Author

etseidl commented Nov 20, 2025

That makes more sense to me if unannotated used to mean nullable. Coming from sql world, adding not null constraints feels more natural 🦕😅

@etseidl
Copy link
Contributor Author

etseidl commented Nov 20, 2025

I'll leave this PR as is, and open a new one with changes to display.

#8890

@alamb
Copy link
Contributor

alamb commented Nov 20, 2025

That makes more sense to me if unannotated used to mean nullable. Coming from sql world, adding not null constraints feels more natural 🦕😅

You and me both 🙏

@etseidl etseidl closed this Nov 20, 2025
@etseidl etseidl deleted the fix_parse_list branch November 20, 2025 20:53
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.

Regression: Parsing List(Int64) results in nullable list in 57.0.0 and a non-nullable list in 57.1.0

2 participants