Re: [PR] IGNITE-28896 SQL Calcite: Support BigDecimal for FETCH, OFFSET and LIMIT [ignite]

["zstan (via GitHub)" <gi...@apache.org>]

zstan commented on code in PR #13375:
URL: https://github.com/apache/ignite/pull/13375#discussion_r3610934293


##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LimitOffsetIntegrationTest.java:
##########
@@ -94,27 +101,80 @@ public class LimitOffsetIntegrationTest extends AbstractBasicIntegrationTransact
     /** */
     @Test
     public void testNestedLimitOffsetWithUnion() {
+        cacheRepl.clear();
+
         sql("INSERT into TEST_REPL VALUES (1, 'a'), (2, 'b'), (3, 'c'), (4, 'd')");
 
         assertQuery("(SELECT id FROM TEST_REPL WHERE id = 2) UNION ALL " +
             "SELECT id FROM (select id from (SELECT id FROM TEST_REPL OFFSET 2) order by id OFFSET 1)"
         ).returns(2).returns(4).check();
     }
 
-    /** Tests correctness of fetch / offset params. */
+    /** */
     @Test
-    public void testInvalidLimitOffset() {
+    public void testFractionalLimitOffset() {
+        cacheRepl.clear();
+
+        sql("INSERT into TEST_REPL VALUES (1, 'a'), (2, 'b'), (3, 'c'), (4, 'd')");
+
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id LIMIT 1.2")
+            .returns(1)
+            .returns(2)
+            .check();
+
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id OFFSET 1.1 ROWS FETCH FIRST 1.1 ROWS ONLY")

Review Comment:
   why 1.1 is rounded to 2 ? it need to be discussed



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java:
##########
@@ -571,7 +576,7 @@ private boolean hasExchange(RelNode rel) {
                 ctx,
                 rowType,
                 idxBndRel.first() ? cmp : cmp.reversed(),
-                0,
+                SortNode.OFFSET_DEFAULT,

Review Comment:
   I specially not touch this in my PR ) What more informative ? 
   node with OFFSET_DEFAULT param - it need to lookup for reviewer or node with concreete "0" param ? 
   Of course it\`s all kinda taste ... I just show you my logic here 



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org