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

Change subject: IMPALA-9924: handle single subquery in or predicate
......................................................................


Patch Set 12:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16152/12/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/16152/12/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@620
PS12, Line 620:         // TODO: IMPALA-9932: we need to recurse over 'expr' 
and systematically pull out
> Is this referencing the right JIRA #?  should this be 9945 ?
I changed it to IMPALA-5226, since that's the parent for making this more 
general.


http://gerrit.cloudera.org:8080/#/c/16152/12/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@627
PS12, Line 627:           parent instanceof BinaryPredicate || parent 
instanceof LikePredicate, parent);
> On second thoughts,  there's not much one can do with Like predicate involv
Yeah, the LIKE support is limited since it breaks if the subquery is an 
argument to concat() or strleft() or whatever. It was just more consistent to 
treat it the same as other predicates.


http://gerrit.cloudera.org:8080/#/c/16152/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java:

http://gerrit.cloudera.org:8080/#/c/16152/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java@892
PS12, Line 892:     // OR predicates nested inside a different different expr 
types in select list
> nit: extra 'different'
Done


http://gerrit.cloudera.org:8080/#/c/16152/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java@897
PS12, Line 897:     // IsNullPredicate rejects nested subqueries.
> For these unsupported cases of nested subqueries , is there an umbrella JIR
I created an epic to aggregate the JIRAs that have already been filed - 
IMPALA-9947 - but I don't think the JIRAs under it completely enumerate the 
gaps.

IMPALA-5226 is now essentially about supporting any conjunct that can be safely 
rewritten into a join, so I added a reference there.

It is pretty confusing what is and isn't supported - I had a lot of surprises 
trying to enumerate interesting cases.


http://gerrit.cloudera.org:8080/#/c/16152/12/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test:

http://gerrit.cloudera.org:8080/#/c/16152/12/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test@4177
PS12, Line 4177: |  other predicates: CASE WHEN id % 2 = 0 THEN id IS NOT NULL 
END OR id = 2
> nit: here the first and last id refer to alltypes table while the middle id
That's easy to fix - it's a fair point.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I64588992901afd7cd885419a0b7f949b0b174976
Gerrit-Change-Number: 16152
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: David Rorke <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Shant Hovsepian <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Sat, 11 Jul 2020 00:27:49 +0000
Gerrit-HasComments: Yes

Reply via email to