Prevent the ORDER BY clause from being dropped in visitor transforms

Fixes #417 Dropping the `ORDER BY` clause prevents the EvaluationException from being thrown by EvaluatingCompiler due to not having evaluation support for ORDER BY, effectively ignoring the ORDER BY clause.
partiql · Jun 15, 2021 · 935756c · 935756c
1 parent 3b2f4ec
commit 935756c
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 16 deletions.
diff --git a/lang/src/org/partiql/lang/eval/EvaluatingCompiler.kt b/lang/src/org/partiql/lang/eval/EvaluatingCompiler.kt
@@ -952,8 +952,9 @@ internal class EvaluatingCompiler(
         selectExpr.orderBy?.let {
             err("ORDER BY is not supported in evaluator yet",
                 ErrorCode.EVALUATOR_FEATURE_NOT_SUPPORTED_YET,
-                errorContextFrom(selectExpr.metas),
-                internal = false )}
+                errorContextFrom(selectExpr.metas).also { it[Property.FEATURE_NAME] = "ORDER BY" },
+                internal = false )
+        }
 
         // Get all the FROM source aliases and LET bindings for binding error checks
         val fold = object : PartiqlAst.VisitorFold<Set<String>>() {

diff --git a/lang/src/org/partiql/lang/eval/visitors/GroupByPathExpressionVisitorTransform.kt b/lang/src/org/partiql/lang/eval/visitors/GroupByPathExpressionVisitorTransform.kt
@@ -98,19 +98,12 @@ class GroupByPathExpressionVisitorTransform(
 
         // The scope of the expressions in the FROM clause is the same as that of the parent scope.
         val from = this.transformExprSelect_from(node)
-
-        val fromLet = node.fromLet?.let { unshadowedTransformer.transformExprSelect_fromLet(node) }
-
-        val where = node.where?.let { unshadowedTransformer.transformExprSelect_where(node) }
-
-        val groupBy = node.group?.let { unshadowedTransformer.transformExprSelect_group(node) }
-
-        val having = node.having?.let { currentAndUnshadowedTransformer.transformExprSelect_having(node) }
-
-        val order = node.having?.let { currentAndUnshadowedTransformer.transformExprSelect_order(node) }
-
-        val limit = node.limit?.let { unshadowedTransformer.transformExprSelect_limit(node) }
-
+        val fromLet = unshadowedTransformer.transformExprSelect_fromLet(node)
+        val where = unshadowedTransformer.transformExprSelect_where(node)
+        val groupBy = unshadowedTransformer.transformExprSelect_group(node)
+        val having = currentAndUnshadowedTransformer.transformExprSelect_having(node)
+        val order = currentAndUnshadowedTransformer.transformExprSelect_order(node)
+        val limit = unshadowedTransformer.transformExprSelect_limit(node)
         val metas = unshadowedTransformer.transformExprSelect_metas(node)
 
         return PartiqlAst.build {

diff --git a/lang/src/org/partiql/lang/eval/visitors/SelectStarVisitorTransform.kt b/lang/src/org/partiql/lang/eval/visitors/SelectStarVisitorTransform.kt
@@ -22,6 +22,7 @@ class SelectStarVisitorTransform : VisitorTransformBase() {
                 where = node.where,
                 group = node.group,
                 having = node.having,
+                order = node.order,
                 limit = node.limit,
                 metas = node.metas)
         }

diff --git a/lang/src/org/partiql/lang/eval/visitors/VisitorTransformBase.kt b/lang/src/org/partiql/lang/eval/visitors/VisitorTransformBase.kt
@@ -40,7 +40,7 @@ abstract class VisitorTransformBase : PartiqlAst.VisitorTransform() {
         val having = transformExprSelect_having(node)
         val setq = transformExprSelect_setq(node)
         val project = transformExprSelect_project(node)
-        val order = node.having?.let { transformExprSelect_order(node) }
+        val order = transformExprSelect_order(node)
         val limit = transformExprSelect_limit(node)
         val metas = transformExprSelect_metas(node)
         return PartiqlAst.build {

diff --git a/lang/test/org/partiql/lang/eval/EvaluatingCompilerTests.kt b/lang/test/org/partiql/lang/eval/EvaluatingCompilerTests.kt
@@ -17,7 +17,10 @@ package org.partiql.lang.eval
 import com.amazon.ion.system.IonSystemBuilder
 import org.junit.Ignore
 import org.junit.Test
+import org.junit.jupiter.api.assertThrows
 import org.partiql.lang.CompilerPipeline
+import org.partiql.lang.errors.ErrorCode
+import org.partiql.lang.errors.Property
 import org.partiql.lang.syntax.ParserException
 
 class EvaluatingCompilerTests : EvaluatorTestBase() {
@@ -1807,4 +1810,14 @@ class EvaluatingCompilerTests : EvaluatorTestBase() {
             AS foo
             """,
             "<< 2 >>")
+
+    @Test
+    fun orderByThrowsCorrectException() {
+        val ex = assertThrows<EvaluationException>("ORDER BY should throw unimplemented exception") {
+            eval("SELECT 1 FROM <<>> ORDER BY x")
+        }
+        assertEquals(ErrorCode.EVALUATOR_FEATURE_NOT_SUPPORTED_YET, ex.errorCode)
+        assertEquals("ORDER BY", ex.errorContext!![Property.FEATURE_NAME]!!.toString())
+    }
+
 }