Skip to content

fix: ensure new_list respects data_type argument - #24029

Open
Ruchirtripathi wants to merge 4 commits into
apache:mainfrom
Ruchirtripathi:fix-scalar-value-new-list
Open

fix: ensure new_list respects data_type argument#24029
Ruchirtripathi wants to merge 4 commits into
apache:mainfrom
Ruchirtripathi:fix-scalar-value-new-list

Conversation

@Ruchirtripathi

@Ruchirtripathi Ruchirtripathi commented Jul 31, 2026

Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

Fixes a bug where ScalarValue::new_list, new_list_nullable, and new_large_list silently ignored the
data_type argument when the values array was non-empty. This caused issues where accumulators like
collect_list could produce outputs with a slightly different type than declared (e.g., in the nullability of
nested fields), leading to invalid argument errors in GroupedHashAggregateStream::emit.

What changes are included in this PR?

  • Added a cast_with_options call for non-empty lists in ScalarValue::new_list, new_list_from_iter, and
    new_large_list.
  • Used DEFAULT_CAST_OPTIONS to ensure the concatenated array is properly reconciled with the requested
    data_type.

Are these changes tested?

Yes, this is covered by existing tests. It resolves the GroupedHashAggregateStream output batch validation
failures for accumulators.

Are there any user-facing changes?

No, this is an internal bug fix.

@github-actions github-actions Bot added the common Related to common crate label Jul 31, 2026
@neilconway

Copy link
Copy Markdown
Contributor

@Ruchirtripathi Thanks for the contribution! Can you take a look at the test failures, please?

@github-actions github-actions Bot added the functions Changes to functions implementation label Jul 31, 2026
@Ruchirtripathi
Ruchirtripathi force-pushed the fix-scalar-value-new-list branch 2 times, most recently from 6bfdcf2 to 55e5063 Compare August 1, 2026 03:35
@Ruchirtripathi
Ruchirtripathi force-pushed the fix-scalar-value-new-list branch from 55e5063 to 1239b46 Compare August 1, 2026 04:05
@Ruchirtripathi

Ruchirtripathi commented Aug 1, 2026

Copy link
Copy Markdown
Author

Hi! I've pushed a new commit that resolves the CI errors while maintaining the strict contract for #24022.
CI checks should be green now. @neilconway Let me know if everything looks good or if you need any further adjustments before merging!

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.85%. Comparing base (f2b4835) to head (a02d17a).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24029      +/-   ##
==========================================
- Coverage   80.85%   80.85%   -0.01%     
==========================================
  Files        1101     1101              
  Lines      375467   375487      +20     
  Branches   375467   375487      +20     
==========================================
+ Hits       303596   303605       +9     
- Misses      53777    53784       +7     
- Partials    18094    18098       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

common Related to common crate functions Changes to functions implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ScalarValue::new_list silently discards its data_type argument for non-empty input

3 participants