Skip to content

fix: report a negative array_resize size as a user error - #25179

Open
1fanwang wants to merge 1 commit into
apache:mainfrom
1fanwang:1fannnw/array-resize-negative-size
Open

fix: report a negative array_resize size as a user error#25179
1fanwang wants to merge 1 commit into
apache:mainfrom
1fanwang:1fannnw/array-resize-negative-size

Conversation

@1fanwang

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

None; found by sweeping the scalar functions with edge-case arguments.

Rationale for this change

array_resize with a negative size raises an internal error, which tells the
caller they hit a DataFusion bug and asks them to open a report:

> SELECT array_resize(make_array(1,2,3), -1, 0);
Internal error: array_resize: failed to convert size to usize.
This issue was likely caused by a bug in DataFusion's code. Please help us to
resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues

The size is an argument to the query, so a negative value is the caller's input
rather than a broken internal invariant. Sending someone to the issue tracker
for their own argument is misleading, and it hides what was actually wrong.

The behaviour was already meant to be an error. array_resize.slt has expected
this call to fail since before this change, but with a bare query error, so
the internal error satisfied it and went unnoticed.

What changes are included in this PR?

Reject a negative size with an execution error naming the value, which matches
how the neighbouring maximum-size check already reports bad input:

> SELECT array_resize(make_array(1,2,3), -1, 0);
Execution error: array_resize: size must not be negative, got -1

Valid input is untouched: 0 still returns [], 5 still grows to
[1, 2, 3, 9, 9], and a NULL size still returns NULL.

Are these changes tested?

Yes. The slt file gains the negative case for both List and LargeList, plus
i64::MIN for the boundary, each pinned to the new message so a regression back
to an internal error fails the suite.

$ cargo test -p datafusion-sqllogictest --test sqllogictests -- array
Progress: 53/53 files completed (100%)

$ cargo test -p datafusion-functions-nested --lib resize
test result: ok. 4 passed; 0 failed

The pre-existing query error case for -5 still passes, so the contract that
a negative size fails is unchanged.

Are there any user-facing changes?

The error for a negative size changes from an internal error to an execution
error with a clear message. No valid call changes behaviour, and no API changes.

The size comes from the query, so a negative value is the caller's input,
not a broken invariant. Raising an internal error told them to file a bug
report for their own argument.

Generated-by: GitHub Copilot CLI (Claude Opus 5)
Signed-off-by: 1fanwang <1fannnw@gmail.com>
@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Sep 10, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.91%. Comparing base (7e5f40a) to head (f25bf2e).

Files with missing lines Patch % Lines
datafusion/functions-nested/src/resize.rs 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25179      +/-   ##
==========================================
- Coverage   81.91%   81.91%   -0.01%     
==========================================
  Files        1132     1132              
  Lines      421117   421119       +2     
  Branches   421117   421119       +2     
==========================================
- Hits       344961   344952       -9     
- Misses      55767    55775       +8     
- Partials    20389    20392       +3     

☔ 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

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants