[CALCITE-7557] Linq4j.ListEnumerable.take(int) / skip(int) diverge from EnumerableDefaults on negative counts

[microbluey]

Linq4j.asEnumerable(list) returns a ListEnumerable, whose list-specialized take(int) and skip(int) pass the count straight to List.subList. A negative count therefore throws, while the generic EnumerableDefaults path returns an empty enumerable (take) or the original sequence (skip):

take(-1): java.lang.IllegalArgumentException: fromIndex(0) > toIndex(-1)
skip(-1): java.lang.IndexOutOfBoundsException: fromIndex = -1

Since ListEnumerable is selected purely as an optimization for List inputs, it should be semantically equivalent to the generic path. This follows the direction agreed in the JIRA discussion: match the existing EnumerableDefaults/LINQ behaviour rather than adopt a fail-fast contract (which would be a broader change across EnumerableDefaults/QueryableDefaults).

Fix

Clamp the count to zero in both methods. Notably, the adjacent take(BigDecimal)/skip(BigDecimal) overloads — added in CALCITE-7624, merged after that discussion — already clamp via count.max(BigDecimal.ZERO). So the int overloads were the only ones in the class not clamping; this change makes them consistent with their own siblings.

Math.max(count, 0) is used rather than negating the count, so that Integer.MIN_VALUE cannot overflow.

Tests

Adds the two reproducers from the JIRA case. Both assert that the generic and list-specialized paths agree.

Verification against main:

• Both new tests fail before the fix (IllegalArgumentException / IndexOutOfBoundsException) and pass after. In each test the first assertion — the generic EnumerableDefaults path — already passed before the fix, confirming this is a genuine divergence between the two paths rather than a bug in both.
• Full ./gradlew build is green: 21599 tests, 0 failures, 0 errors across all modules.
• :linq4j:style (checkstyle + autostyle) passes.

I also checked two edge cases beyond the ticket, which are not in the committed tests:

input	ListEnumerable	EnumerableDefaults
take(Integer.MIN_VALUE)	[]	[]
skip(Integer.MIN_VALUE)	[1, 2, 3]	[1, 2, 3]
take(0) / skip(0)	unchanged	unchanged

take(count >= size) still returns this, preserving the existing identity optimization.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[mihaibudiu]

Can this be reflected in some SQL tests as well? What happens for SELECT * LIMIT -2.5?

[microbluey]

Good question — I tried it, and the answer is that negative counts can't reach this code path from SQL at all, which is why I kept the tests at the linq4j level.

Three cases against fetch.iq:

select * from (values (1),(2),(3)) as t(x) fetch next (-2.5) rows only;
  --> IllegalArgumentException: FETCH must not be negative

select * from (values (1),(2),(3)) as t(x) limit -2;
  --> parse failed: Encountered "-" at line 1, column 52

select * from (values (1),(2),(3)) as t(x) offset -2 rows;
  --> parse failed: Encountered "-" at line 1, column 53

So SQL is guarded twice over: the parser takes UnsignedNumericLiteralOrParam() for OFFSET/FETCH, so a negative literal is a syntax error; and for a parenthesized expression that evaluates negative, EnumUtils (must not be negative, EnumUtils.java:147) rejects it before any enumerable is built. LIMIT -2.5 therefore never reaches ListEnumerable.take.

What this PR fixes is the linq4j API used directly, where EnumerableDefaults and the list-specialized path disagree for the same negative input. Since SQL can't produce that state, a quidem test would only be asserting the parser/validator behaviour above — which seems worth having, but as a separate concern from this divergence. Happy to add those three cases to fetch.iq in this PR if you'd like them pinned down; just say the word and I'll push a commit.

[mihaibudiu]

I don't know if microbluey is a robot, but it would be really nice to have shorter answers. Can you please edit down the AI generated answers to the essential parts?

[microbluey]

Sorry mihaibudiu — I was asleep. I'm sorry it came across that way. Shorter answers:

• LIMIT -2.5 → rejected before it reaches this code: "FETCH must not be negative". LIMIT -2 / OFFSET -2 don't even parse. So no SQL test can cover this divergence — it's only reachable through linq4j directly.
• quidem for [CALCITE-7487] ProjectJoinTransposeRule throws ArrayIndexOutOfBoundsException in PushProjector when a Join input has a zero-column row type #5125 → no. I tried; the validator never produces a zero-column join input. Only RelBuilder or an external frontend can (the reporter used substrait-java).
• DEE → the relation with no attributes and one tuple; standard relational algebra but not used anywhere in Calcite, so I replaced it with a plain description in 269e562.