Repository: asterixdb Updated Branches: refs/heads/master 0a7233d8f -> 83695b932
[ASTERIXDB-2248][SQLPP] Disallow use of column aliases in GBY/HAVING - user model changes: yes - storage format changes: no - interface changes: no Details: - Column aliases defined by SELECT clause should not be referenceable from GROUP BY/HAVING clauses because aliases are not variables. References from ORDER BY clause are still allowed. Change-Id: Ieb734f87904e6bf307a04b2328a2fd109151f70f Reviewed-on: https://asterix-gerrit.ics.uci.edu/2294 Sonar-Qube: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> Reviewed-by: Till Westmann <[email protected]> Contrib: Till Westmann <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/asterixdb/repo Commit: http://git-wip-us.apache.org/repos/asf/asterixdb/commit/83695b93 Tree: http://git-wip-us.apache.org/repos/asf/asterixdb/tree/83695b93 Diff: http://git-wip-us.apache.org/repos/asf/asterixdb/diff/83695b93 Branch: refs/heads/master Commit: 83695b932440628a58148f2551b776503f55c5d3 Parents: 0a7233d Author: Dmitry Lychagin <[email protected]> Authored: Wed Jan 17 14:53:46 2018 -0800 Committer: Dmitry Lychagin <[email protected]> Committed: Fri Jan 19 14:17:39 2018 -0800 ---------------------------------------------------------------------- .../parserts/queries_sqlpp/columnalias.sqlpp | 8 +- .../parserts/queries_sqlpp/columnalias3.sqlpp | 8 +- .../results_parser_sqlpp/columnalias.ast | 80 ++++++++++++++------ .../results_parser_sqlpp/columnalias3.ast | 80 ++++++++++++++------ .../queries_sqlpp/tpcds/q24a/q24a.3.query.sqlpp | 3 +- .../queries_sqlpp/tpcds/q24b/q24b.3.query.sqlpp | 3 +- .../resources/runtimets/testsuite_sqlpp.xml | 3 + .../visitor/InlineColumnAliasVisitor.java | 18 ++--- 8 files changed, 130 insertions(+), 73 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/asterixdb/blob/83695b93/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias.sqlpp ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias.sqlpp b/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias.sqlpp index 72eb1d9..9d1af80 100644 --- a/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias.sqlpp +++ b/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias.sqlpp @@ -17,8 +17,6 @@ * under the License. */ -SELECT SQRT(t.a*t.b) AS root FROM tbl_name t -GROUP BY root -WITH u AS root -HAVING root > 0 -ORDER BY u; \ No newline at end of file +SELECT sum(t.a*t.b) AS root FROM tbl_name t +GROUP BY t.id +ORDER BY root; \ No newline at end of file http://git-wip-us.apache.org/repos/asf/asterixdb/blob/83695b93/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias3.sqlpp ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias3.sqlpp b/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias3.sqlpp index 8226362..d95b42d 100644 --- a/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias3.sqlpp +++ b/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias3.sqlpp @@ -17,8 +17,6 @@ * under the License. */ -SELECT ELEMENT {'root': SQRT(t.a*t.b)} FROM tbl_name t -GROUP BY root -WITH u AS root -HAVING root > 0 -ORDER BY u; \ No newline at end of file +SELECT ELEMENT {'root': SUM(t.a*t.b)} FROM tbl_name t +GROUP BY t.id +ORDER BY root; \ No newline at end of file http://git-wip-us.apache.org/repos/asf/asterixdb/blob/83695b93/asterixdb/asterix-app/src/test/resources/parserts/results_parser_sqlpp/columnalias.ast ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/test/resources/parserts/results_parser_sqlpp/columnalias.ast b/asterixdb/asterix-app/src/test/resources/parserts/results_parser_sqlpp/columnalias.ast index 9c5aaf5..1e0efba 100644 --- a/asterixdb/asterix-app/src/test/resources/parserts/results_parser_sqlpp/columnalias.ast +++ b/asterixdb/asterix-app/src/test/resources/parserts/results_parser_sqlpp/columnalias.ast @@ -1,44 +1,76 @@ Query: SELECT [ -Variable [ Name=$root ] -root -] -FROM [ FunctionCall asterix.dataset@1[ - LiteralExpr [STRING] [tbl_name] - ] - AS Variable [ Name=$t ] -] -Groupby - Variable [ Name=$root ] - := - FunctionCall null.sqrt@1[ +FunctionCall asterix.sql-sum@1[ + ( + SELECT ELEMENT [ OperatorExpr [ FieldAccessor [ - Variable [ Name=$t ] + FieldAccessor [ + Variable [ Name=#3 ] + Field=t + ] Field=a ] * FieldAccessor [ - Variable [ Name=$t ] + FieldAccessor [ + Variable [ Name=#3 ] + Field=t + ] Field=b ] ] + ] + FROM [ Variable [ Name=#1 ] + AS Variable [ Name=#3 ] + ] + ) +] +root +] +FROM [ FunctionCall asterix.dataset@1[ + LiteralExpr [STRING] [tbl_name] + ] + AS Variable [ Name=$t ] +] +Groupby + Variable [ Name=$id ] + := + FieldAccessor [ + Variable [ Name=$t ] + Field=id ] GROUP AS Variable [ Name=#1 ] ( t:=Variable [ Name=$t ] ) -Let Variable [ Name=$u ] - := - Variable [ Name=$root ] - HAVING - OperatorExpr [ - Variable [ Name=$root ] - > - LiteralExpr [LONG] [0] - ] Orderby - Variable [ Name=$u ] + FunctionCall asterix.sql-sum@1[ + ( + SELECT ELEMENT [ + OperatorExpr [ + FieldAccessor [ + FieldAccessor [ + Variable [ Name=#2 ] + Field=t + ] + Field=a + ] + * + FieldAccessor [ + FieldAccessor [ + Variable [ Name=#2 ] + Field=t + ] + Field=b + ] + ] + ] + FROM [ Variable [ Name=#1 ] + AS Variable [ Name=#2 ] + ] + ) + ] ASC http://git-wip-us.apache.org/repos/asf/asterixdb/blob/83695b93/asterixdb/asterix-app/src/test/resources/parserts/results_parser_sqlpp/columnalias3.ast ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/test/resources/parserts/results_parser_sqlpp/columnalias3.ast b/asterixdb/asterix-app/src/test/resources/parserts/results_parser_sqlpp/columnalias3.ast index 6db92c9..7deb117 100644 --- a/asterixdb/asterix-app/src/test/resources/parserts/results_parser_sqlpp/columnalias3.ast +++ b/asterixdb/asterix-app/src/test/resources/parserts/results_parser_sqlpp/columnalias3.ast @@ -4,7 +4,32 @@ RecordConstructor [ ( LiteralExpr [STRING] [root] : - Variable [ Name=$root ] + FunctionCall asterix.sql-sum@1[ + ( + SELECT ELEMENT [ + OperatorExpr [ + FieldAccessor [ + FieldAccessor [ + Variable [ Name=#3 ] + Field=t + ] + Field=a + ] + * + FieldAccessor [ + FieldAccessor [ + Variable [ Name=#3 ] + Field=t + ] + Field=b + ] + ] + ] + FROM [ Variable [ Name=#1 ] + AS Variable [ Name=#3 ] + ] + ) + ] ) ] ] @@ -14,36 +39,43 @@ FROM [ FunctionCall asterix.dataset@1[ AS Variable [ Name=$t ] ] Groupby - Variable [ Name=$root ] + Variable [ Name=$id ] := - FunctionCall null.sqrt@1[ - OperatorExpr [ - FieldAccessor [ - Variable [ Name=$t ] - Field=a - ] - * - FieldAccessor [ - Variable [ Name=$t ] - Field=b - ] - ] + FieldAccessor [ + Variable [ Name=$t ] + Field=id ] GROUP AS Variable [ Name=#1 ] ( t:=Variable [ Name=$t ] ) -Let Variable [ Name=$u ] - := - Variable [ Name=$root ] - HAVING - OperatorExpr [ - Variable [ Name=$root ] - > - LiteralExpr [LONG] [0] - ] Orderby - Variable [ Name=$u ] + FunctionCall asterix.sql-sum@1[ + ( + SELECT ELEMENT [ + OperatorExpr [ + FieldAccessor [ + FieldAccessor [ + Variable [ Name=#2 ] + Field=t + ] + Field=a + ] + * + FieldAccessor [ + FieldAccessor [ + Variable [ Name=#2 ] + Field=t + ] + Field=b + ] + ] + ] + FROM [ Variable [ Name=#1 ] + AS Variable [ Name=#2 ] + ] + ) + ] ASC http://git-wip-us.apache.org/repos/asf/asterixdb/blob/83695b93/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpcds/q24a/q24a.3.query.sqlpp ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpcds/q24a/q24a.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpcds/q24a/q24a.3.query.sqlpp index c24c10e..95e4e8c 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpcds/q24a/q24a.3.query.sqlpp +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpcds/q24a/q24a.3.query.sqlpp @@ -59,12 +59,13 @@ GROUP BY c.c_last_name SELECT c_last_name ,c_first_name ,s_store_name - ,SUM(netpaid) paid + ,paid FROM ssales WHERE i_color = 'orchid' GROUP BY c_last_name ,c_first_name ,s_store_name GROUP AS g +LET paid = SUM(netpaid) HAVING paid > (SELECT value 0.05*avg(g.ssales.netpaid) FROM g)[0] ; \ No newline at end of file http://git-wip-us.apache.org/repos/asf/asterixdb/blob/83695b93/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpcds/q24b/q24b.3.query.sqlpp ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpcds/q24b/q24b.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpcds/q24b/q24b.3.query.sqlpp index 86ffa7b..3a1590d 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpcds/q24b/q24b.3.query.sqlpp +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpcds/q24b/q24b.3.query.sqlpp @@ -59,12 +59,13 @@ GROUP BY c.c_last_name SELECT c_last_name ,c_first_name ,s_store_name - ,SUM(netpaid) paid + ,paid FROM ssales WHERE i_color = 'chiffon' GROUP BY c_last_name ,c_first_name ,s_store_name GROUP AS g +LET paid = SUM(netpaid) HAVING paid > (SELECT value 0.05*avg(g.ssales.netpaid) FROM g)[0] ; \ No newline at end of file http://git-wip-us.apache.org/repos/asf/asterixdb/blob/83695b93/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml index 0b8fd8c..16fa334 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -1563,6 +1563,7 @@ <test-case FilePath="dapd"> <compilation-unit name="q2-3"> <output-dir compare="Text">q2</output-dir> + <expected-error>Cannot resolve ambiguous alias reference for undefined identifier sig_id</expected-error> </compilation-unit> </test-case> <test-case FilePath="dapd"> @@ -2901,11 +2902,13 @@ <test-case FilePath="group-by"> <compilation-unit name="sugar-02"> <output-dir compare="Text">core-02</output-dir> + <expected-error>Cannot resolve ambiguous alias reference for undefined identifier deptId</expected-error> </compilation-unit> </test-case> <test-case FilePath="group-by"> <compilation-unit name="sugar-02-2"> <output-dir compare="Text">core-02</output-dir> + <expected-error>Cannot resolve ambiguous alias reference for undefined identifier deptId</expected-error> </compilation-unit> </test-case> <test-case FilePath="group-by"> http://git-wip-us.apache.org/repos/asf/asterixdb/blob/83695b93/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java index 628bf57..270b366 100644 --- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java +++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java @@ -28,7 +28,6 @@ import org.apache.asterix.lang.common.base.Expression; import org.apache.asterix.lang.common.base.Expression.Kind; import org.apache.asterix.lang.common.base.ILangExpression; import org.apache.asterix.lang.common.base.Literal; -import org.apache.asterix.lang.common.clause.LetClause; import org.apache.asterix.lang.common.expression.FieldBinding; import org.apache.asterix.lang.common.expression.LiteralExpr; import org.apache.asterix.lang.common.expression.RecordConstructor; @@ -45,6 +44,11 @@ import org.apache.asterix.lang.sqlpp.util.SqlppVariableUtil; import org.apache.asterix.lang.sqlpp.visitor.SqlppSubstituteExpressionVisitor; import org.apache.asterix.lang.sqlpp.visitor.base.AbstractSqlppExpressionScopingVisitor; +/** + * Syntactic sugar rewriting: inlines column aliases defines in SELECT clause into ORDER BY and LIMIT clauses. <br/> + * Note: column aliases are not cosidered new variables, but they can be referenced from ORDER BY and LIMIT clauses + * because of this rewriting (like in SQL) + */ public class InlineColumnAliasVisitor extends AbstractSqlppExpressionScopingVisitor { public InlineColumnAliasVisitor(LangRewritingContext context) { @@ -67,18 +71,6 @@ public class InlineColumnAliasVisitor extends AbstractSqlppExpressionScopingVisi // Creates a substitution visitor. SqlppSubstituteExpressionVisitor visitor = new SqlppSubstituteExpressionVisitor(context, map); - // Rewrites GROUP BY/LET/HAVING clauses. - if (selectBlock.hasGroupbyClause()) { - selectBlock.getGroupbyClause().accept(visitor, arg); - } - if (selectBlock.hasLetClausesAfterGroupby()) { - for (LetClause letClause : selectBlock.getLetListAfterGroupby()) { - letClause.accept(visitor, arg); - } - } - if (selectBlock.hasHavingClause()) { - selectBlock.getHavingClause().accept(visitor, arg); - } SelectExpression selectExpression = (SelectExpression) arg; // For SET operation queries, column aliases will not substitute ORDER BY nor LIMIT expressions.
