Allow DESC ordering in window expressions (#15195) (#15221)

(cherry picked from commit fbbb9c7)
apache · Oct 20, 2023 · b6c306f · b6c306f
1 parent 4882fb0
commit b6c306f
Show file tree

Hide file tree

Showing 6 changed files with 153 additions and 92 deletions.
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
@@ -22,7 +22,14 @@
 import org.apache.calcite.adapter.java.JavaTypeFactory;
 import org.apache.calcite.prepare.BaseDruidSqlValidator;
 import org.apache.calcite.prepare.CalciteCatalogReader;
+import org.apache.calcite.runtime.CalciteContextException;
+import org.apache.calcite.runtime.CalciteException;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlOperatorTable;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
 
 /**
  * Druid extended SQL validator. (At present, it doesn't actually
@@ -39,4 +46,33 @@ protected DruidSqlValidator(
   {
     super(opTab, catalogReader, typeFactory, validatorConfig);
   }
+
+  @Override
+  public void validateCall(SqlCall call, SqlValidatorScope scope)
+  {
+    if (call.getKind() == SqlKind.NULLS_FIRST) {
+      SqlNode op0 = call.getOperandList().get(0);
+      if (op0.getKind() == SqlKind.DESCENDING) {
+        throw buildCalciteContextException("DESCENDING ordering with NULLS FIRST is not supported!", call);
+      }
+    }
+    if (call.getKind() == SqlKind.NULLS_LAST) {
+      SqlNode op0 = call.getOperandList().get(0);
+      if (op0.getKind() != SqlKind.DESCENDING) {
+        throw buildCalciteContextException("ASCENDING ordering with NULLS LAST is not supported!", call);
+      }
+    }
+    super.validateCall(call, scope);
+  }
+
+  private CalciteContextException buildCalciteContextException(String message, SqlCall call)
+  {
+    SqlParserPos pos = call.getParserPosition();
+    return new CalciteContextException(message,
+        new CalciteException(message, null),
+        pos.getLineNum(),
+        pos.getColumnNum(),
+        pos.getEndLineNum(),
+        pos.getEndColumnNum());
+  }
 }
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/convertlet/DruidConvertletTable.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/convertlet/DruidConvertletTable.java
@@ -67,6 +67,9 @@ public class DruidConvertletTable implements SqlRexConvertletTable
                    .add(SqlStdOperatorTable.NULLIF)
                    .add(SqlStdOperatorTable.COALESCE)
                    .add(SqlLibraryOperators.NVL)
+                   .add(SqlStdOperatorTable.DESC)
+                   .add(SqlStdOperatorTable.NULLS_FIRST)
+                   .add(SqlStdOperatorTable.NULLS_LAST)
                    .build();
 
   private final Map<SqlOperator, SqlRexConvertlet> table;

diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -137,6 +137,9 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertThrows;
+
 public class CalciteQueryTest extends BaseCalciteQueryTest
 {
   @Test
@@ -14251,4 +14254,25 @@ public void testLatestByOnStringColumnWithoutMaxBytesSpecified()
             new Object[] {"abc", defaultString, "def", defaultString, "def", defaultString}
         ));
   }
+
+  @Test
+  public void testUnSupportedNullsFirst()
+  {
+    DruidException e = assertThrows(DruidException.class, () -> testBuilder()
+        .queryContext(ImmutableMap.of(PlannerContext.CTX_ENABLE_WINDOW_FNS, true))
+        .sql("SELECT dim1,ROW_NUMBER() OVER (ORDER BY dim1 DESC NULLS FIRST) from druid.foo")
+        .run());
+
+    assertThat(e, invalidSqlIs("DESCENDING ordering with NULLS FIRST is not supported! (line [1], column [41])"));
+  }
+
+  @Test
+  public void testUnSupportedNullsLast()
+  {
+    DruidException e = assertThrows(DruidException.class, () -> testBuilder()
+        .queryContext(ImmutableMap.of(PlannerContext.CTX_ENABLE_WINDOW_FNS, true))
+        .sql("SELECT dim1,ROW_NUMBER() OVER (ORDER BY dim1 NULLS LAST) from druid.foo")
+        .run());
+    assertThat(e, invalidSqlIs("ASCENDING ordering with NULLS LAST is not supported! (line [1], column [41])"));
+  }
 }