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-1422: support a constant on LHS of IN predicates.
Date Wed, 25 Oct 2017 18:42:31 GMT
Dimitris Tsirogiannis has posted comments on this change. (

Change subject: IMPALA-1422: support a constant on LHS of IN predicates.

Patch Set 5:


Did you add any planner tests?
File fe/src/main/java/org/apache/impala/analysis/
PS5, Line 228: 	  if (newConjunct instanceof BoolLiteral) {
             : 	      smap.put(conjunct, newConjunct);
             : 	      continue;
             : 	  }
nit: remove tabs. Also, I am not sure I understand what is happening here.
PS5, Line 308: correlated aggregates
what is a correlated aggregate?
PS5, Line 323: c
use capital C just to be consistent with L321
PS5, Line 329: 10 IS NULL OR
Since we know what the constant literal is, can't we avoid the is null check and the disjunction
in some cases?
PS5, Line 355: or 10 IS NOT NULL
Same comment as above.
PS5, Line 360: .
nit: remove '.'
PS5, Line 366: 
nit: remove empty lines (L366, L369 and 371)
PS5, Line 428: newSubquery.reset();
             :       newSubquery.analyze(rhsQuery.getAnalyzer());
Why do you need to go through reset/analyze? Same question for L459-460.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d69889a3c72e90be9d4ccf47d2816819ae32acb
Gerrit-Change-Number: 8322
Gerrit-PatchSet: 5
Gerrit-Owner: Vuk Ercegovac <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Vuk Ercegovac <>
Gerrit-Comment-Date: Wed, 25 Oct 2017 18:42:31 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message