This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/main by this push:
     new 657f871bc2 [CALCITE-6759] SqlToRelConverter should not remove ORDER BY 
in subquery if it has an OFFSET
657f871bc2 is described below

commit 657f871bc28780eb91162e6231a35e00298f6a9a
Author: suibianwanwan <[email protected]>
AuthorDate: Sat Jan 4 19:27:45 2025 +0800

    [CALCITE-6759] SqlToRelConverter should not remove ORDER BY in subquery if 
it has an OFFSET
    
    While an ORDER BY on its own can be ignored, an ORDER BY with
    an OFFSET or FETCH cannot be removed from the subquery without
    changing the semantics.
    
    Close apache/calcite#4119
---
 .../java/org/apache/calcite/plan/RelOptUtil.java   |  6 +--
 .../apache/calcite/sql2rel/SqlToRelConverter.java  |  2 +-
 .../apache/calcite/test/SqlToRelConverterTest.java | 16 +++++++
 .../apache/calcite/test/SqlToRelConverterTest.xml  | 17 +++++++
 core/src/test/resources/sql/sub-query.iq           | 52 ++++++++++++++++++++++
 5 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java 
b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index 76e37434ab..5a45ef15e2 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -189,10 +189,10 @@ public abstract class RelOptUtil {
   }
 
   /**
-   * Whether this node is a sort without limit specification.
+   * Whether this node is a sort with neither limit nor offset specification.
    */
   public static boolean isPureOrder(RelNode rel) {
-    return !isLimit(rel) && isOrder(rel);
+    return !isLimit(rel) && !isOffset(rel) && isOrder(rel);
   }
 
   /**
@@ -210,7 +210,7 @@ public abstract class RelOptUtil {
   }
 
   /**
-   * Whether this node contains a offset specification.
+   * Whether this node contains an offset specification.
    */
   public static boolean isOffset(RelNode rel) {
     return (rel instanceof Sort) && ((Sort) rel).offset != null;
diff --git 
a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java 
b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
index e4fe01b2fc..9bb2794858 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -775,7 +775,7 @@ public class SqlToRelConverter {
     // Semantics example. Given the view definition
     //   CREATE VIEW v2 AS SELECT * FROM t ORDER BY x LIMIT 10
     // we would never remove the ORDER BY, because "ORDER BY ... LIMIT" is 
about
-    // semantics. It is not a 'pure order'.
+    // semantics. It is not a 'pure order'. Similarly "ORDER BY x OFFSET 5".
     if (RelOptUtil.isPureOrder(castNonNull(bb.root))
         && config.isRemoveSortInSubQuery()) {
       // Remove the Sort if the view is at the top level. Also remove the Sort
diff --git 
a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java 
b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index d97a73f163..f2a7cbb911 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -5177,6 +5177,22 @@ class SqlToRelConverterTest extends SqlToRelTestBase {
     sql(sql).withConfig(c -> 
c.withRemoveSortInSubQuery(false)).convertsTo("${planKeepSort}");
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6759";>[CALCITE-6759]
+   * SqlToRelConverter should not remove ORDER BY in subquery if it has an
+   * OFFSET</a>.
+   *
+   * <p>While an ORDER BY on its own can be ignored, an ORDER BY with an OFFSET
+   * or FETCH cannot be removed from the subquery without changing the
+   * semantics. */
+  @Test void testSortWithOffsetInSubQuery() {
+    final String sql = "select count(*) from (\n"
+        + "  select *\n"
+        + "  from emp\n"
+        + "  order by empno offset 10)";
+    sql(sql).ok();
+  }
+
   @Test void testTrimUnionAll() {
     final String sql = ""
         + "select deptno from\n"
diff --git 
a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml 
b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
index 49a6200e7d..35baf36dcd 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -7565,6 +7565,23 @@ LogicalSort(sort0=[$1], dir0=[DESC], fetch=[3])
           LogicalTableScan(table=[[CATALOG, SALES, EMP]])
       LogicalProject(ENAME=[$1], SAL=[$5], DEPTNO=[$7])
         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testSortWithOffsetInSubQuery">
+    <Resource name="sql">
+      <![CDATA[select count(*) from (
+  select *
+  from emp
+  order by empno offset 10)]]>
+    </Resource>
+    <Resource name="plan">
+      <![CDATA[
+LogicalAggregate(group=[{}], EXPR$0=[COUNT()])
+  LogicalProject($f0=[0])
+    LogicalSort(sort0=[$0], dir0=[ASC], offset=[10])
+      LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], 
HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
+        LogicalTableScan(table=[[CATALOG, SALES, EMP]])
 ]]>
     </Resource>
   </TestCase>
diff --git a/core/src/test/resources/sql/sub-query.iq 
b/core/src/test/resources/sql/sub-query.iq
index 248ec165ba..35ce9cbb55 100644
--- a/core/src/test/resources/sql/sub-query.iq
+++ b/core/src/test/resources/sql/sub-query.iq
@@ -149,6 +149,58 @@ select deptno, deptno not in (select deptno from emp where 
deptno is not null) f
 
 !ok
 
+# [CALCITE-6579] SqlToRelConverter should not remove ORDER BY in subquery if 
it has an OFFSET
+select count(*) as c from (select * from emp order by deptno offset 4);
++---+
+| C |
++---+
+| 5 |
++---+
+(1 row)
+
+!ok
+
+# Subquery with ORDER BY and LIMIT
+select count(*) as c from (select * from emp order by deptno limit 3);
++---+
+| C |
++---+
+| 3 |
++---+
+(1 row)
+
+!ok
+
+# Subquery with ORDER BY and LIMIT. (LIMIT exceeds row count so has no effect.)
+select count(*) as c from (select * from emp order by deptno limit 99);
++---+
+| C |
++---+
+| 9 |
++---+
+(1 row)
+
+!ok
+
+# Previous queries as scalar subqueries.
+select
+  (select count(*) from emp) as c,
+  (select count(*) from (select * from emp order by deptno)) as co,
+  (select count(*) from (select * from emp order by deptno offset 4)) as o4,
+  (select count(*) from (select * from emp order by deptno limit 3)) as l3,
+  (select count(*) from (select * from emp order by deptno limit 99)) as l99,
+  (select count(*) from (select * from emp order by deptno limit 99 offset 
94)) as l99o94,
+  (select count(*) from (select * from emp order by deptno limit 99 offset 5)) 
as l99o5
+from (values 1) as t (v);
++---+----+----+----+-----+--------+-------+
+| C | CO | O4 | L3 | L99 | L99O94 | L99O5 |
++---+----+----+----+-----+--------+-------+
+| 9 |  9 |  5 |  3 |   9 |      0 |     4 |
++---+----+----+----+-----+--------+-------+
+(1 row)
+
+!ok
+
 # RHS has no rows
 # Even 'NULL NOT IN ...' is TRUE.
 select * from dept where deptno not in (select deptno from emp where false);

Reply via email to