Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9005 )
Change subject: IMPALA-6314: Add run time scalar subquery check for uncorrelated subqueries ...................................................................... Patch Set 15: (7 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/9005/13/be/src/exec/cardinality-check-node.cc File be/src/exec/cardinality-check-node.cc: http://gerrit.cloudera.org:8080/#/c/9005/13/be/src/exec/cardinality-check-node.cc@90 PS13, Line 90: > We should technically also set num_rows_returned_ (it's used in a couple of Done http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java File fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java: http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java@175 PS11, Line 175: if (!operand.type_.isNumericType() && !operand.type_.isNull()) { > Good question, thanks for the detailed explanation. The following alternati I like this suggestion, it does make things much easier and simple. However, I'm getting a few strange errors that I couldn't really fix. Some say "select list expression not produced by aggregation output", others complain about the memory estimation, but I don't really see the connection to my changes. I've ran the frontend tests and the backend tests that I touched. Frontend tests produce 8 failures and 3 errors, tests/query_test/test_nested_types.py::TestNestedTypes::test_subplan also fails. http://gerrit.cloudera.org:8080/#/c/9005/13/fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java File fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java: http://gerrit.cloudera.org:8080/#/c/9005/13/fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java@32 PS13, Line 32: m > extra space Done http://gerrit.cloudera.org:8080/#/c/9005/9/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: http://gerrit.cloudera.org:8080/#/c/9005/9/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1116 PS9, Line 1116: // place. > Looking at the example query again, I think it's ok that it succeeds. We're I removed the quick fix from InlineViewRef.analyze() http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java: http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@1434 PS11, Line 1434: > Thanks! Works for me. Done http://gerrit.cloudera.org:8080/#/c/9005/13/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test File testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test: http://gerrit.cloudera.org:8080/#/c/9005/13/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test@709 PS13, Line 709: Subquery must not return more than one row: SELECT o_orderkey FROM c.c_orders WHERE o_orderkey = 6000000 OR o_orderkey = 4285920 > Is it possible to include the query test in the error message so we can be Done http://gerrit.cloudera.org:8080/#/c/9005/13/testdata/workloads/functional-query/queries/QueryTest/subquery.test File testdata/workloads/functional-query/queries/QueryTest/subquery.test: http://gerrit.cloudera.org:8080/#/c/9005/13/testdata/workloads/functional-query/queries/QueryTest/subquery.test@841 PS13, Line 841: functional > Let's drop the functional. prefix, it's not required - the idea is that we I get "Could not resolve table reference: ..." errors if I do that. The other queries in this file are also prefixed with the database. Should I migrate the queries to a different file where the test matrix is used? -- To view, visit http://gerrit.cloudera.org:8080/9005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0f52b93a60eeacedd242a2f17fa6b99c4fc38e06 Gerrit-Change-Number: 9005 Gerrit-PatchSet: 15 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 21 Mar 2018 23:08:47 +0000 Gerrit-HasComments: Yes
