Alex Behm has posted comments on this change. ( )

Change subject: IMPALA-6314: Add run time scalar subquery check for 
uncorrelated subqueries

Patch Set 23:


Code looks pretty good now. I'll do another pass over everything focusing on 
the tests.
File fe/src/main/java/org/apache/impala/analysis/
PS21, Line 1025:     if (isCorrelated && 
!item.getExpr().contains(Expr.IS_BUILTIN_AGG_FN)) {
> This allows runtime scalars here, because their type is scalar now and not
Right. That could be an indication that this check is useless or that we have a 
gap in test coverage. I'm worried about the latter. One of two outcomes is 
* add a test that hits this branch
* convince ourselves that the type is always scalar here, and then add a 
Preconditions check
File fe/src/test/java/org/apache/impala/analysis/
PS23, Line 82:       if (node instanceof StatementBase) {
No need for this. We're always going to have a StatementBase
PS23, Line 917:     //TODO: make it work again or delete
              : //    testToSql("select * from functional.alltypes where not 
(id < 10 and " +
              : //            "(int_col in (select int_col from 
functional.alltypestiny)) and " +
              : //            "(string_col = (select max(string_col) from 
              : //        "SELECT * FROM functional.alltypes WHERE NOT (id < 10 
AND " +
              : //            "(int_col IN (SELECT int_col FROM 
functional.alltypestiny)) AND " +
              : //        "(string_col = (SELECT max(string_col) FROM 
> So, is it OK if I just leave this TODO here?
Please file an improvement JIRA for this case, and remove the test.

The issue here is that we push the NOT into the condition, so the ANDs turn 
into ORs and then we no longer support the subquery.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f52b93a60eeacedd242a2f17fa6b99c4fc38e06
Gerrit-Change-Number: 9005
Gerrit-PatchSet: 23
Gerrit-Owner: Zoltan Borok-Nagy <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Attila Jeges <>
Gerrit-Reviewer: Csaba Ringhofer <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Reviewer: Vuk Ercegovac <>
Gerrit-Reviewer: Zoltan Borok-Nagy <>
Gerrit-Comment-Date: Wed, 11 Apr 2018 21:44:28 +0000
Gerrit-HasComments: Yes

Reply via email to