You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2023/06/08 21:16:17 UTC

[calcite] branch main updated: [CALCITE-5768] Fix early return in hasSortByOrdinal

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 0ea24a04ee [CALCITE-5768] Fix early return in hasSortByOrdinal
0ea24a04ee is described below

commit 0ea24a04ee5168a7605472bb5ab93686fc6e7c36
Author: Will Noble <wn...@google.com>
AuthorDate: Wed Jun 7 18:16:59 2023 -0700

    [CALCITE-5768] Fix early return in hasSortByOrdinal
---
 .../org/apache/calcite/rel/rel2sql/SqlImplementor.java | 12 +++++++-----
 .../calcite/rel/rel2sql/RelToSqlConverterTest.java     | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
index f04a5a11cf..e90e5dacbc 100644
--- a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
+++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
@@ -1898,12 +1898,14 @@ public abstract class SqlImplementor {
           return false;
         }
         for (SqlNode sqlNode : orderList) {
-          if (!(sqlNode instanceof SqlBasicCall)) {
-            return sqlNode instanceof SqlNumericLiteral;
+          if (sqlNode instanceof SqlNumericLiteral) {
+            return true;
           }
-          for (SqlNode operand : ((SqlBasicCall) sqlNode).getOperandList()) {
-            if (operand instanceof SqlNumericLiteral) {
-              return true;
+          if (sqlNode instanceof SqlBasicCall) {
+            for (SqlNode operand : ((SqlBasicCall) sqlNode).getOperandList()) {
+              if (operand instanceof SqlNumericLiteral) {
+                return true;
+              }
             }
           }
         }
diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index 412ddb4c16..9de19094a8 100644
--- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -400,6 +400,24 @@ class RelToSqlConverterTest {
     relFn(relFn).ok(expected);
   }
 
+  @Test void testUsesSubqueryWhenSortingByIdThenOrdinal() {
+    final Function<RelBuilder, RelNode> relFn = b -> b
+        .scan("EMP")
+        .aggregate(
+            b.groupKey("JOB"),
+            b.aggregateCall(SqlStdOperatorTable.COUNT, b.field("ENAME")))
+        .sort(b.field(0), b.field(1))
+        .project(b.field(0))
+        .build();
+    final String expected = "SELECT \"JOB\"\n"
+        + "FROM (SELECT \"JOB\", COUNT(\"ENAME\") AS \"$f1\"\n"
+        + "FROM \"scott\".\"EMP\"\n"
+        + "GROUP BY \"JOB\"\n"
+        + "ORDER BY \"JOB\", 2) AS \"t0\"";
+
+    relFn(relFn).ok(expected);
+  }
+
   @Test void testSelectQueryWithWhereClauseOfBasicOperators() {
     String query = "select * from \"product\" "
         + "where (\"product_id\" = 10 OR \"product_id\" <= 5) "