microbluey commented on PR #5124:
URL: https://github.com/apache/calcite/pull/5124#issuecomment-5094150396
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.
--
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]
