Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3861: Replace BetweenPredicates with their equivalent CompoundPredicate. ......................................................................
Patch Set 3: Code-Review+1 (2 comments) Nice! Brute force simple solution :) http://gerrit.cloudera.org:8080/#/c/3804/3/fe/src/main/java/com/cloudera/impala/analysis/StmtRewriter.java File fe/src/main/java/com/cloudera/impala/analysis/StmtRewriter.java: PS3, Line 279: replaceBetweenPredicates Do you think it would make more sense to call this function rewriteBetweenPredicates()? http://gerrit.cloudera.org:8080/#/c/3804/3/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeSubqueriesTest.java File fe/src/test/java/com/cloudera/impala/analysis/AnalyzeSubqueriesTest.java: PS3, Line 223: a.int_col between t.tinyint_col and t.bigint_col For completion, you may want to add a case where the column from the outer block in a correlated between predicate is on the left of the between (e.g. t.tinyint_col between a.int_col and a.tinyint_col). Another interesting case, which we should theoretically handle is: t.tinyint_col between a.int_col and t.int_col (I don't know if a normal person would write something like that :)). -- To view, visit http://gerrit.cloudera.org:8080/3804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0838b30444ed9704ce6a058d30718a24caa7444a Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Internal Jenkins Gerrit-HasComments: Yes
