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

Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................


Patch Set 6:

(2 comments)

Had a few questions about why this works to help me convince myself that it's 
correct.

http://gerrit.cloudera.org:8080/#/c/14813/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14813/6//COMMIT_MSG@2
PS6, Line 2: Author:     Aman Sinha <[email protected]>
I dunno if this matters, but it seems like you have two different email 
addresses configured in git (here and l4)


http://gerrit.cloudera.org:8080/#/c/14813/6/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/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1201
PS6, Line 1201:     analyzer.markConjunctsAssigned(preds);
I'm trying to convince myself that it's correct to mark the conjuncts as 
assigned here if we remove them from the list below. The potential issue is that

I'm not sure what the right option is but I think there's something to clarify 
or fix. I can think of a few alternatives, I think we probably need to at least 
update a comment or two.
* Maybe we need to mention somewhere that it's safe to drop inferred predicates 
(like identity predicates) because the original predicates will still be 
assigned. I think this is true.
* We could also potentially remove the predicates from 'preds' up here. Does 
that have other consequences? E.g. the predicate stays around and gets assigned 
somewhere. Or is the inferred predicate getting removed key to the fix?
* Is is possible we should just not be generating the inferred conjuncts? E.g. 
Analyzer.createEquivConjuncts(TupleId, List<T>,
      Set<SlotId>) should not be creating these.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Mon, 09 Dec 2019 16:16:46 +0000
Gerrit-HasComments: Yes

Reply via email to