Till Westmann has posted comments on this change. Change subject: [ASTERIXDB-2170][SQL] Fix resolution order of implicit field access ......................................................................
Patch Set 6: (11 comments) https://asterix-gerrit.ics.uci.edu/#/c/2207/6//COMMIT_MSG Commit Message: PS6, Line 15: nearest variable in scope Could we add a description of "the scope" to the issue? https://asterix-gerrit.ics.uci.edu/#/c/2207/6/asterixdb/asterix-app/src/test/resources/optimizerts/queries_sqlpp/count-tweets.sqlpp File asterixdb/asterix-app/src/test/resources/optimizerts/queries_sqlpp/count-tweets.sqlpp: PS6, Line 42: t as t, Do we need "t" here? https://asterix-gerrit.ics.uci.edu/#/c/2207/6/asterixdb/asterix-app/src/test/resources/optimizerts/results_parser_sqlpp/fj-dblp-csx.ast File asterixdb/asterix-app/src/test/resources/optimizerts/results_parser_sqlpp/fj-dblp-csx.ast: PS6, Line 95: Huh? https://asterix-gerrit.ics.uci.edu/#/c/2207/6/asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias2.sqlpp File asterixdb/asterix-app/src/test/resources/parserts/queries_sqlpp/columnalias2.sqlpp: PS6, Line 20: sum This is confusing, what did this query do before? https://asterix-gerrit.ics.uci.edu/#/c/2207/6/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/resolution/groupby_rename_with_sugar/groupby_rename_with_sugar.1.ddl.sqlpp File asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/resolution/groupby_rename_with_sugar/groupby_rename_with_sugar.1.ddl.sqlpp: PS6, Line 24: UNION ALL What happened to the union_negative_2 queries? https://asterix-gerrit.ics.uci.edu/#/c/2207/6/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpch-with-index/q13_customer_distribution/q13_customer_distribution.3.query.sqlpp File asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpch-with-index/q13_customer_distribution/q13_customer_distribution.3.query.sqlpp: PS6, Line 27: SUM The support for sugars in different places seems to have changed. Do the sugars work in both places or is this the only option now? https://asterix-gerrit.ics.uci.edu/#/c/2207/6/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpch/query-issue562/query-issue562.3.query.sqlpp File asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpch/query-issue562/query-issue562.3.query.sqlpp: PS6, Line 37: : : : : : Why did this go? https://asterix-gerrit.ics.uci.edu/#/c/2207/6/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/union/union_order_by_5/union_orderby_5.3.query.sqlpp File asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/union/union_order_by_5/union_orderby_5.3.query.sqlpp: PS6, Line 30: t.id, id What do these resolve to? https://asterix-gerrit.ics.uci.edu/#/c/2207/6/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml File asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml: PS6, Line 2790: function field-access-by-name Unrelated to this change, but I think that "field-access-by-name" is nothing a user would write, so we might want to adjust the error message (maybe we can file an issue for this). https://asterix-gerrit.ics.uci.edu/#/c/2207/6/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/InsertStatement.java File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/InsertStatement.java: Line 119: return null; > MAJOR SonarQube violation: Sounds like a good suggestion. Is that feasible in this context? https://asterix-gerrit.ics.uci.edu/#/c/2207/6/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java: PS6, Line 269: } : else formatting -- To view, visit https://asterix-gerrit.ics.uci.edu/2207 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I50bc823ff53da06507a5454b30f4f500b862d4bf Gerrit-PatchSet: 6 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Steven Jacobs <sjaco...@ucr.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-HasComments: Yes
