microbluey opened a new pull request, #5124: URL: https://github.com/apache/calcite/pull/5124
`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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
