Alex Behm has posted comments on this change.

Change subject: IMPALA-5531: Fix correctness issue in correlated aggregate 
subqueries
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7706/1/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
File fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java:

Line 681:             List<Expr> slotRefs = Lists.newArrayList();
Might be easier/shorter:
 
List<TupleId> tids = Lists.newArrayList();
getIds(tids, null);
return !Collections.disjoint(tids, subqueryTblIds);


Line 690:     // Inequality correlated predicates are not currently supported 
in aggregate
This bug is not specific to inequality predicates. I think it's really that we 
only support equality predicates. For example, try queries like this:

select 1 from functional.alltypes t1
where int_col = (select max(id) from functional.alltypes t2 where t1.id = t2.id 
and coalesce(t1.bool_col, t2.bool_col));

select 1 from functional.alltypes t1
where int_col = (select max(id) from functional.alltypes t2 where t1.id = t2.id 
and t1.string_col like t2.string_col);

Those queries fail in other mysterious ways, but the point is that this bug is 
not specific to inequality.


Line 691:     // subqueries (see IMPALA-5531).
We should be able to run queries with arbitrary correlated predicates if the 
subquery references relative nested collections.


Line 692:     if (expr instanceof BinaryPredicate && 
!correlatedPredicates.isEmpty() &&
Let's report which predicate makes the subquery unsupported.

Also the BinaryPredicate check is redundant with Expr.IS_NEQ_BINARY_PREDICATE.


-- 
To view, visit http://gerrit.cloudera.org:8080/7706
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ca7b60ef0543430d2f5a802285254ebb52db2ab
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-HasComments: Yes

Reply via email to