Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-1767 Adds predicate to test boolean values true, false, unknown. ......................................................................
Patch Set 3: (10 comments) http://gerrit.cloudera.org:8080/#/c/8014/3/fe/src/main/cup/sql-parser.cup File fe/src/main/cup/sql-parser.cup: PS3, Line 1583: whitespace PS3, Line 3034: whitespace, here and below http://gerrit.cloudera.org:8080/#/c/8014/3/fe/src/main/java/org/apache/impala/analysis/BoolTestPredicate.java File fe/src/main/java/org/apache/impala/analysis/BoolTestPredicate.java: PS3, Line 26: Class describing a predicate that tests a boolean values using IS. : * Example: (1 < 1) IS TRUE You should explain more thoroughly what this expr actually does, eg. the handling of NULLs. PS3, Line 28: a : * CompoundPredicate weird line wrapping here PS3, Line 33: _ We don't use trailing underscores on constant values. PS3, Line 94: assert Preconditions.checkState http://gerrit.cloudera.org:8080/#/c/8014/3/fe/src/main/java/org/apache/impala/rewrite/BoolTestToCompoundRule.java File fe/src/main/java/org/apache/impala/rewrite/BoolTestToCompoundRule.java: PS3, Line 39: if (!(expr instanceof BoolTestPredicate)) : return expr; single line PS3, Line 64: private BoolTestToCompoundRule() { : } single line http://gerrit.cloudera.org:8080/#/c/8014/3/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java: PS3, Line 164: whitespace http://gerrit.cloudera.org:8080/#/c/8014/3/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java: Line 1257: // BoolTestPredicate. modify one of these tests to use "NOT" -- 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: 3 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
