Shant Hovsepian has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16007 )

Change subject: IMPALA-8984: Uncorrelated scalar subqueries in the select list
......................................................................


Patch Set 4:

(9 comments)

Some more tests and review comments addressed. Still want to get a good test 
run out of jenkins.

http://gerrit.cloudera.org:8080/#/c/16007/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:

http://gerrit.cloudera.org:8080/#/c/16007/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@292
PS3, Line 292:                 "Invariant violated: Only subqueries that are 
guaranteed to return a "
> nit:  "guaranteed"
Done


http://gerrit.cloudera.org:8080/#/c/16007/3/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
File fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java:

http://gerrit.cloudera.org:8080/#/c/16007/3/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@937
PS3, Line 937:      * supported in the FROM clause, WHERE clause and SELECT 
list. The rewrite is
> Update this comment for SELECT clause.
Done


http://gerrit.cloudera.org:8080/#/c/16007/3/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@1137
PS3, Line 1137:      *    returned per group so a run time cardinality check 
must be applied. An exception
> nit: duplicate 'primary'
Done


http://gerrit.cloudera.org:8080/#/c/16007/4/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
File fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java:

http://gerrit.cloudera.org:8080/#/c/16007/4/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@1117
PS4, Line 1117:      *
Done


http://gerrit.cloudera.org:8080/#/c/16007/4/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@1117
PS4, Line 1117:      *
Done


http://gerrit.cloudera.org:8080/#/c/16007/4/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@1135
PS4, Line 1135:      *
Done


http://gerrit.cloudera.org:8080/#/c/16007/4/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@1230
PS4, Line 1230:           // rewrite to a LOJ.
I added a test for this. I know it feels weird but since the slotref for the 
subquery is marked as materialized and the other join queries get bound by the 
USING/ON clause, nothing explodes. Since there are scalar subqueries, the only 
weird situation is if the cardinality of all the joins where equal to 1 then it 
might get reordered but the results would still be correct.

If it were a correlated subquery then we've need to handle things more 
carefully, but that's for a later commit.


http://gerrit.cloudera.org:8080/#/c/16007/4/testdata/workloads/functional-query/queries/QueryTest/subquery.test
File testdata/workloads/functional-query/queries/QueryTest/subquery.test:

http://gerrit.cloudera.org:8080/#/c/16007/4/testdata/workloads/functional-query/queries/QueryTest/subquery.test@1044
PS4, Line 1044: select id, 1+(select min(id) from functional.alltypessmall)
Added the tpc-ds query, it's a pretty complex plan.


http://gerrit.cloudera.org:8080/#/c/16007/4/testdata/workloads/tpcds/queries/tpcds-decimal_v2-q9.test
File testdata/workloads/tpcds/queries/tpcds-decimal_v2-q9.test:

http://gerrit.cloudera.org:8080/#/c/16007/4/testdata/workloads/tpcds/queries/tpcds-decimal_v2-q9.test@3
PS4, Line 3: select case when (select count(*)
Done



--
To view, visit http://gerrit.cloudera.org:8080/16007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibcf55d26889aa01d69bb85f18c9241dda095fb66
Gerrit-Change-Number: 16007
Gerrit-PatchSet: 4
Gerrit-Owner: Shant Hovsepian <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Shant Hovsepian <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Thu, 25 Jun 2020 19:38:11 +0000
Gerrit-HasComments: Yes

Reply via email to