tkalkirill commented on PR #5050: URL: https://github.com/apache/calcite/pull/5050#issuecomment-4806520537
@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. -- 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]
