Preserve input field nullability in ArrayAgg return field #19868
+284
−23
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
This PR attempts to preserve the input field's nullability in the ArrayAgg aggregate function's return field. When the input field is non-nullable, the result field would be marked as non-nullable; when the input is nullable, the result field would remain nullable.
Changes:
- Added a
return_fieldmethod override to theArrayAggimplementation that sets the result field's nullability to match the input field's nullability - Added a unit test to verify that the schema metadata correctly reflects the input field's nullability
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
GaneshPatil7517
force-pushed
the
feature/arrayagg-preserve-nullability
branch
from
668ce8c to
b833445
Compare
GaneshPatil7517
force-pushed
the
feature/arrayagg-preserve-nullability
branch
from
114f5bc to
2b2a41b
Compare
Jefffrey
reviewed
Jan 17, 2026
- Override return_field() to preserve input field nullability in list elements - Add input_nullable field to all ArrayAgg accumulators - List itself remains nullable (NULL for empty groups) - Inner elements nullability now matches input field nullability
GaneshPatil7517
force-pushed
the
feature/arrayagg-preserve-nullability
branch
from
024785b to
24e57cb
Compare
- Separate state() from evaluate() in all three accumulators - state() always uses nullable=true for inner elements (matches state_fields()) - evaluate() uses input_nullable to preserve nullability for final output - Fix clippy: use std::slice::from_ref() instead of .clone() on FieldRef
- Fix array_sort return_type() to preserve input inner field nullability - Fix clippy: use Arc::clone() instead of .clone() on FieldRef - Update unnest_with_redundant_columns snapshot for non-nullable inner
Jefffrey
reviewed
Jan 19, 2026
|
|
||
| fn state(&mut self) -> Result<Vec<ScalarValue>> { | ||
| Ok(vec![self.evaluate()?]) | ||
| // State uses nullable inner elements to match state_fields() schema |
Contributor
There was a problem hiding this comment.
This seems like something we should be fixing in state_fields() instead of having this handling code
| impl Accumulator for DistinctArrayAggAccumulator { | ||
| fn state(&mut self) -> Result<Vec<ScalarValue>> { | ||
| Ok(vec![self.evaluate()?]) | ||
| // State uses nullable inner elements to match state_fields() schema |
| } | ||
|
|
||
| let mut result = vec![self.evaluate()?]; | ||
| // State uses nullable inner elements to match state_fields() schema |
- Make return_type() return error since return_field() is implemented - Simplify comments in return_field() - Revert array_sort changes (unrelated to original issue) - Revert spark/collect.rs changes (UDAF return_field not fixed)
GaneshPatil7517
force-pushed
the
feature/arrayagg-preserve-nullability
branch
from
84c066f to
402927a
Compare
Contributor
Author
|
ok ill update this all |
Contributor
|
@GaneshPatil7517 Please refrain from closing a discussion when no action has been taken or no comment was left. My comments regarding the state related functions don't seem to have been addressed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Fixes #19768
Rationale for this change
Currently,
ArrayAggalways produces an output with data type: