impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-1767 Adds predicate to test boolean values true, false, unknown.
Date Wed, 13 Sep 2017 17:45:37 GMT
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-1767 Adds predicate to test boolean values true, false, unknown.

Patch Set 7:

File fe/src/main/java/org/apache/impala/analysis/

PS7, Line 333: Bool tests
File fe/src/main/java/org/apache/impala/analysis/

PS7, Line 28: <bool_expr> IS [NOT] <val>
Replace with the one in L31.

PS7, Line 30: <val> is one of (TRUE | FALSE | UNKNOWN).
Remove, it's kind of redundant.
File fe/src/test/java/org/apache/impala/analysis/

PS6, Line 591: 
> added negative tests and coverage to rewrites and end-to-end. the analysis 
Sorry, I don't think I understand this comment. What do you mean by analysis for the expr
does not do much? Aren't the negative test cases captured by AnalysisError()? I think rewrite
tests should handle the rewrite logic but everything else (analysis related) should be tested
here. Maybe I am missing something.
File fe/src/test/java/org/apache/impala/analysis/

PS7, Line 217: // Incorrect type for LHS.
             :     AnalyzedRewritesError("'foo' is true", rule, "'foo' = TRUE AND 'foo' IS
             :         "operands of type STRING and BOOLEAN are not comparable: 'foo' = TRUE");
             :     // No subqueries allowed.
             :     RewritesError(
             :         "(select max(tinyint_col) from functional.alltypessmall) is true",
             :         "Subqueries are not supported in the select list.");
Hm, it's not clear to me why this should be here and not as an Analysis test.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bd0aa82a0316ac236b7ecf8612ef4b30c016a00
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vuk Ercegovac <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Philip Zeyliger <>
Gerrit-Reviewer: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Vuk Ercegovac <>
Gerrit-HasComments: Yes

View raw message