Alex Behm has posted comments on this change. Change subject: IMPALA-3167: Fix assignment of WHERE-clause predicate through grouping agg + outer join. ......................................................................
Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/4960/2//COMMIT_MSG Commit Message: Line 7: IMPALA-3167: Fix assignment of WHERE-clause predicate through grouping agg + outer join. > long lines I though we allowed up to 90 chars. If that's the case then our gerrit is misconfigured. Fixed anyway. Line 28: The fix is to conservatively not mark bound predicates as assigned if there exists > if there are Done Line 30: duplicate assignments of predicates. Those are simply deduped now. > i don't understand that last part. Expanded commit msg with an example that shows how duplicate assignment can happen. http://gerrit.cloudera.org:8080/#/c/4960/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: Line 1548: (evalAfterJoin(srcConjunct) > move to preceding line and fix indentation, like l1551 Done Line 1550: != globalState_.outerJoinedTupleIds.get(srcTid))) > fix indentation Done Line 1553: != globalState_.outerJoinedTupleIds.get(destTid))); > same here Done http://gerrit.cloudera.org:8080/#/c/4960/2/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: Line 1194: Expr.removeDuplicates(conjuncts); > how do you end up with duplicate predicates in a *scan* node due to the cha Explained duplicates in commit msg with an example. Your point about HDFS scans only is a good one. Looks like we don't pick up bound predicates in other scan nodes! That's a bug. I think we should address that very soon, at least for Kudu. But I'd prefer to separate those changes from this bugfix. -- To view, visit http://gerrit.cloudera.org:8080/4960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I774d13a13ad1e8fe82512df98dc29983bdd232eb Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Anonymous Coward #27 Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-HasComments: Yes