From 2b672f6e0d9857e6a150f0800a889a2095538705 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Tue, 15 Jun 2021 13:01:33 -0700 Subject: [PATCH] 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/lang/eval/EvaluatingCompiler.kt | 5 +++-- .../GroupByPathExpressionVisitorTransform.kt | 19 ++++++------------- .../visitors/SelectStarVisitorTransform.kt | 1 + .../eval/visitors/VisitorTransformBase.kt | 2 +- .../lang/eval/EvaluatingCompilerTests.kt | 13 +++++++++++++ 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/lang/src/org/partiql/lang/eval/EvaluatingCompiler.kt b/lang/src/org/partiql/lang/eval/EvaluatingCompiler.kt index 0b1a9ca655..f1f814e1a2 100644 --- 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>() { diff --git a/lang/src/org/partiql/lang/eval/visitors/GroupByPathExpressionVisitorTransform.kt b/lang/src/org/partiql/lang/eval/visitors/GroupByPathExpressionVisitorTransform.kt index 799642c221..7128d7d558 100644 --- 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 index fddf7d8459..8a3b59876b 100644 --- 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 index bf4fff5c6c..5f7a976a3f 100644 --- 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 index 1d3f745955..3d6e3f5cb2 100644 --- 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("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()) + } + }