Alex Behm has posted comments on this change. Change subject: IMPALA-5597: Check predicate children types when building runtime filter plan ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/7949/1/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java: Line 240: if (!targetExpr.getType().matchesType(srcExpr.getType())) return null; We can still fix this case, see my comments below. Line 585: targetExpr = targetExpr.substitute(smap, analyzer, true); Looks like the existing code is trying to do the right thing, but it's not quite correct. Since our runtime filters are hash based (bloom) the source and target exprs must have *identical* types, otherwise the same values may hash differently. Proposed changes: 1. Pass 'false' to this substitute() to not preserve the type. 2. Try to cast the resulting targetExpr to filter.getSrcExpr().getType(). If that fails, we have to bail like in the existing catch block here. 3. The Preconditions check in L597 is too weak. We require the types to be identical (use equals()). -- To view, visit http://gerrit.cloudera.org:8080/7949 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0d66673e280ce13cd3a514236c0c2ecbc50475a6 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi Wang <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-HasComments: Yes
