Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16052 )

Change subject: IMPALA-9784: Non correlated subqueries in HAVING.
......................................................................


Patch Set 2:

(8 comments)

I think this makes sense, doing the nested query thing and converting it to a 
where clause does simplify it a lot.

http://gerrit.cloudera.org:8080/#/c/16052/2/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/16052/2/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@465
PS2, Line 465:         // TODO: Remove this when independent subquery 
evaluation is implemented.
Can we add a simple planner test for this rewrite? With the cardinality check 
node added.


http://gerrit.cloudera.org:8080/#/c/16052/2/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@466
PS2, Line 466:         // TODO: IMPALA-5100 to cover all cases, we do let 
through runtime scalars with
> I relaxed some of these rules to let through subqueries such as (select cou
It might be helpful to file a separate JIRA with an example of this query, just 
so we have some record that we can run some new queries. Some simple things 
like:

  select * from functional.alltypes where id < (select count(bool_col) from 
functional.alltypes where int_col=1 group by int_col);


http://gerrit.cloudera.org:8080/#/c/16052/2/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@469
PS2, Line 469: presensce
nit: presence


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

http://gerrit.cloudera.org:8080/#/c/16052/2/testdata/workloads/functional-query/queries/QueryTest/subquery.test@1218
PS2, Line 1218:   from functional.alltypestiny group by id
nit: remove the functional database name here and below - the test framework 
should set the current db based on the file format in the test matrix.. A lot 
of other queries here don't follow the best practice.


http://gerrit.cloudera.org:8080/#/c/16052/2/testdata/workloads/tpcds/queries/tpcds-q23-1.test
File testdata/workloads/tpcds/queries/tpcds-q23-1.test:

http://gerrit.cloudera.org:8080/#/c/16052/2/testdata/workloads/tpcds/queries/tpcds-q23-1.test@52
PS2, Line 52: ---- RESULTS
Compared to 
https://github.com/gregrahn/tpcds-kit/blob/master/answer_sets/23_NULLS_LAST.ans


http://gerrit.cloudera.org:8080/#/c/16052/2/testdata/workloads/tpcds/queries/tpcds-q23-2.test
File testdata/workloads/tpcds/queries/tpcds-q23-2.test:

http://gerrit.cloudera.org:8080/#/c/16052/2/testdata/workloads/tpcds/queries/tpcds-q23-2.test@59
PS2, Line 59: 'Brown','Monika',6031.52
Compared to 
https://github.com/gregrahn/tpcds-kit/blob/master/answer_sets/23_NULLS_LAST.ans


http://gerrit.cloudera.org:8080/#/c/16052/2/testdata/workloads/tpcds/queries/tpcds-q24-2.test
File testdata/workloads/tpcds/queries/tpcds-q24-2.test:

http://gerrit.cloudera.org:8080/#/c/16052/2/testdata/workloads/tpcds/queries/tpcds-q24-2.test@53
PS2, Line 53: 'Hamlin','Heather','able',149.65
Compared to https://github.com/gregrahn/tpcds-kit/blob/master/answer_sets/24.ans


http://gerrit.cloudera.org:8080/#/c/16052/2/testdata/workloads/tpcds/queries/tpcds-q44.test
File testdata/workloads/tpcds/queries/tpcds-q44.test:

http://gerrit.cloudera.org:8080/#/c/16052/2/testdata/workloads/tpcds/queries/tpcds-q44.test@51
PS2, Line 51: 1,'oughtantiprin st','callyeingbarcallyought'
Compared this to 
https://github.com/gregrahn/tpcds-kit/blob/master/answer_sets/44.ans



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I124a58a09a1a47e1222a22d84b54fe7d07844461
Gerrit-Change-Number: 16052
Gerrit-PatchSet: 2
Gerrit-Owner: Shant Hovsepian <sh...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Shant Hovsepian <sh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Jun 2020 19:35:58 +0000
Gerrit-HasComments: Yes

Reply via email to