Tim Armstrong 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 13: (4 comments) Backend changes look good to me still. 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: COUNTER_SET(rows_returned_counter_, 1); We should technically also set num_rows_returned_ (it's used in a couple of places, like be/src/exec/partitioned-aggregation-node.cc). 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: extra space 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: Is it possible to include the query test in the error message so we can be sure the error message is correct. 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 can run against different file formats in the test matrix automatically. -- 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: 13 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: Tue, 13 Mar 2018 22:19:54 +0000 Gerrit-HasComments: Yes
