Alex Behm has posted comments on this change. Change subject: IMPALA-3065: Register !empty() predicates as On-clause conjuncts. ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/2365/1/fe/src/main/java/com/cloudera/impala/analysis/CollectionTableRef.java File fe/src/main/java/com/cloudera/impala/analysis/CollectionTableRef.java: Line 103: // an empty result set. > comment doesn't apply anymore. I think is still applies because we are hoping that the predicate is picked up in the parent scan. I changed the comment to clarify. Line 108: // affect the result of this join (e.g., and not the whole FROM clause). > hm, that's a bummer, it would be nice to register it directly as a scan pre I think it would be wrong to do that. We need to do the usual analysis to see if the predicate can be correctly assigned below outer and anti joins. The tests have such cases. http://gerrit.cloudera.org:8080/#/c/2365/1/testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test File testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test: Line 685: | | predicates: !empty(a.struct_array_col) > what's the point of having it here? would the nlj even return anything if i You are correct. There is no point in having it here in this case. But there is also no harm (results are empty anyway). Our code currently does not really distinguish whether 'a' is outer joined by this join, by a join below or above. So if this NLJ was an outer join, we could not remove the predicate. We'd need to introduce special casing to remove the predicate in this case, so I'm not sure it's worth it. -- To view, visit http://gerrit.cloudera.org:8080/2365 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7236940057f6468b085ffb2c228c3146b102cea7 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-2.5.0_5.7.0 Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
