Vuk Ercegovac has posted comments on this change. Change subject: IMPALA-1767 Adds predicate to test boolean values true, false, unknown. ......................................................................
Patch Set 8: (5 comments) http://gerrit.cloudera.org:8080/#/c/8014/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: PS7, Line 333: BooTestPre > BoolTestPredicates? Done http://gerrit.cloudera.org:8080/#/c/8014/7/fe/src/main/java/org/apache/impala/analysis/BoolTestPredicate.java File fe/src/main/java/org/apache/impala/analysis/BoolTestPredicate.java: PS7, Line 28: <expr> IS [NOT] <val>. > Replace with the one in L31. Done PS7, Line 30: ool, compatible with bool or null > Remove, it's kind of redundant. Done http://gerrit.cloudera.org:8080/#/c/8014/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java: PS6, Line 591: for (String r > Sorry, I don't think I understand this comment. What do you mean by analysi ignore that comment. tests are now included here. http://gerrit.cloudera.org:8080/#/c/8014/7/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java: PS7, Line 217: "(bool_col is null and string_col = '10' and int_col < 10)", rule, : "int_col < 10 AND bool_col IS NULL AND " + : "((bigint_col < 10) OR (string_col = '10'))"); : // Negated common conjunct: !(int_col=5 or tinyint_col > 9) : RewritesOk( : "(!(int_col=5 or tinyint_col > 9) and double_col = 7) or " + : "(!(int_col=5 or tinyint_col > 9) and double_col = 8 > Hm, it's not clear to me why this should be here and not as an Analysis tes agreed, I understood incorrectly that analysis tests also run rewrites and that these rewrite tests are just for testing the result of rewrites, not including the subsequent analysis of the rewritten expression. removed these tests. -- To view, visit http://gerrit.cloudera.org:8080/8014 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5bd0aa82a0316ac236b7ecf8612ef4b30c016a00 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-HasComments: Yes
