Skip to content

Fix array_lower/array_upper to respect custom lower bounds#36780

Merged
antiguru merged 5 commits into
mainfrom
claude/peaceful-ride-681zU
May 29, 2026
Merged

Fix array_lower/array_upper to respect custom lower bounds#36780
antiguru merged 5 commits into
mainfrom
claude/peaceful-ride-681zU

Conversation

@antiguru
Copy link
Copy Markdown
Member

@antiguru antiguru commented May 29, 2026

Motivation

Closes https://github.com/MaterializeInc/database-issues/issues/11278

Both array_lower() and array_upper() ignored the array's custom lower bound:

  • array_lower() always returned 1, regardless of the array's actual lower bound.
  • array_upper() returned the dimension's length rather than its upper bound (lower_bound + length - 1).

For arrays with the default lower bound of 1 these happen to be correct (since upper_bound == length), so the bug only surfaced for arrays with a non-default lower bound, e.g. those built by array_fill():

SELECT array_lower(array_fill(0, ARRAY[3], ARRAY[5]), 1);  -- returned 1, should be 5
SELECT array_upper(array_fill(0, ARRAY[3], ARRAY[5]), 1);  -- returned 3, should be 7

PostgreSQL returns 5 and 7 respectively.

Description

  • array_lower(): now returns the dimension's actual lower bound via dim.dimension_bounds() instead of hardcoding 1. The return type changed from Option<i32> to Result<Option<i32>, EvalError> (matching array_upper()) so a bound that does not fit in i32 is reported as an error.
  • array_upper(): now returns the dimension's upper bound via dim.dimension_bounds() instead of dim.length.
  • Reduced the sqlfunc attribute lists on both functions to just is_infix_op = true; output_type, sqlname, propagates_nulls, and introduces_nulls are inferred by the macro from the signature and matched the previous explicit values.

Verification

  • Added a unit test (array_lower_upper_respect_lower_bound) in src/expr/src/scalar/func.rs covering default, custom positive, negative, and out-of-range dimensions for both functions.
  • Added test cases in test/sqllogictest/array_fill.slt for single-dimension custom (5), negative (-3), and multi-dimensional (4, 3) lower bounds.

https://claude.ai/code/session_01A4oEvWwfEb6yE8FYgPAfpQ

claude added 3 commits May 29, 2026 02:39
array_lower hardcoded a return value of 1 and array_upper returned the
dimension length, both ignoring the array's actual lower bound. For
arrays created with a non-default lower bound (e.g. via array_fill with a
lower-bounds argument), this produced wrong results that disagreed with
Postgres.

Both now derive their results from ArrayDimension::dimension_bounds, so
array_lower returns the lower bound and array_upper returns
lower_bound + length - 1.

Fixes https://github.com/MaterializeInc/database-issues/issues/11278

https://claude.ai/code/session_01A4oEvWwfEb6yE8FYgPAfpQ
Comment thread test/sqllogictest/array_fill.slt Outdated
Comment thread src/expr/src/scalar/func.rs Outdated
Comment thread src/expr/src/scalar/func.rs Outdated
claude added 2 commits May 29, 2026 11:35
Remove is_infix_op from array_lower/array_upper so they render as
function calls (e.g. array_lower(arr, 1)) in EXPLAIN, consistent with
array_length. Also drop the issue-number reference from the slt comment.

https://claude.ai/code/session_01A4oEvWwfEb6yE8FYgPAfpQ
Reduce the sqlfunc attribute lists for array_lower/array_upper to just
is_infix_op = true; output_type, sqlname, propagates_nulls and
introduces_nulls are all inferred by the macro from the function
signature, matching the previous explicit values.

https://claude.ai/code/session_01A4oEvWwfEb6yE8FYgPAfpQ
@antiguru antiguru changed the title Fix array_lower to respect custom lower bounds Fix array_lower/array_upper to respect custom lower bounds May 29, 2026
@antiguru antiguru marked this pull request as ready for review May 29, 2026 15:12
@antiguru antiguru requested a review from a team as a code owner May 29, 2026 15:12
@antiguru antiguru requested review from DAlperin and def- May 29, 2026 15:14
Copy link
Copy Markdown
Member

@DAlperin DAlperin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@antiguru antiguru merged commit 1a4a268 into main May 29, 2026
119 checks passed
@antiguru antiguru deleted the claude/peaceful-ride-681zU branch May 29, 2026 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants