[CALCITE-7624] Support BigDecimal for FETCH and OFFSET in Enumerable

[tkalkirill]

Jira Link

CALCITE-7624

Changes Proposed

This PR proposes using BigDecimal for FETCH and OFFSET, so that the following SQL queries work:

SELECT * FROM person OFFSET 1.5 ROWS FETCH NEXT 1.5 ROWS ONLY;
SELECT * FROM person ORDER BY name OFFSET 1.5 ROWS FETCH NEXT 1.5 ROWS ONLY;

SELECT * FROM person OFFSET ? ROWS FETCH NEXT ? ROWS ONLY;
SELECT * FROM person ORDER BY name OFFSET ? ROWS FETCH NEXT ? ROWS ONLY;
--?1 - int, long, BigDecimal
--?2 - int, long, BigDecimal

[tkalkirill]

@xiedeyantu Can you take a look?
I have a few questions:

1. Regarding negative values, I noticed that if OFFSET/FETCH are specified as literals, a validation error occurs; however, if passed as a parameter, execution proceeds without errors. At first glance, this seems inconsistent; perhaps the parameters themselves should also be checked and trigger an error.

2. For org.apache.calcite.linq4j.ExtendedEnumerable, should we add skip/take methods that accept long/BigDecimal arguments? In my opinion, it is worth adding them, but with default behavior that uses EnumerableDefaults.

3. It is now possible to pass a fractional value such as 1.5 for OFFSET/FETCH. In Oracle, the fractional part is truncated (resulting in 1.0), whereas in PostgreSQL, it is rounded to the nearest integer (resulting in 2.0). I verified this behavior for both databases in https://onecompiler.com/. Currently, our implementation matches the PostgreSQL behavior. Perhaps we could handle this using a dialect-based approach?

4. Should the documentation (visible in https://calcite.apache.org/docs/reference.html) be updated? If so, how is that done? Is it a separate task, part of this PR, or handled some other way?

5. Regarding java.sql.ParameterMetaData: there is a test that checks for the parameters types org.apache.calcite.test.JdbcTest#checkPreparedOffsetFetch- shouldn't that be changed to DECIMAL/NUMERIC? On one hand, this might break backward compatibility; on the other, it introduces inconsistency. I'm not sure how to handle this.

6. What about the adapters do they need to support BigDecimal? In my view, they do.

Of course, I could create a task for each question and resolve them there. Is that standard practice, or should I address them via the dev list first?

[sonarqubecloud]

Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.4% Coverage on New Code
7.2% Duplication on New Code

See analysis details on SonarQube Cloud

[mihaibudiu]

In general we should push as much as possible in the validation and code generation layers.
If the runtime does not support fractional values, the code generation should generate a rounding operation.
I would pick a reasonable implementation and do that, if other people need different semantics, they can file issues or override behaviors. Every new configuration flag increases exponentially the test surface, so I would avoid them unless they are really needed.

[xiedeyantu]

@tkalkirill Thanks for spinning this feature out into a separate piece of work!

For the first point, I agree with Mihai’s suggestion—we should intercept invalid formats as early as possible in the pre-processing logic, and we can also refer to how errors are gracefully handled in the existing execution flow. This would also serve as good groundwork for your earlier PR.

For the second point, I think we should add something similar to what you implemented in your previous PR. I’m wondering whether we should deprecate the old approach and support only  BigDecimal  going forward? This might be worth continuing to discuss in the Jira ticket.

On the third point, I also agree with Mihai. We could consider following PostgreSQL’s behavior, since Calcite often takes Postgres as a reference first; if Postgres doesn’t support something, then other mainstream implementations are considered. That said, if you believe another approach is more reasonable, as long as we document the rationale clearly where needed, that should be acceptable.

For the fourth point, I do think it makes sense to allow updates.

Regarding the fifth point, I personally feel we should unify the behavior—widening the type shouldn’t break compatibility.

On the sixth point, for adapters, I think we could either apply different validation logic per backend or let each backend handle its own checks. Both options seem reasonable to me; let’s see if others have better ideas. If not, as long as we provide accurate documentation, that should be fine.