Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9005 )
Change subject: IMPALA-6314: Add run time scalar subquery check for uncorrelated subqueries ...................................................................... Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java File fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java: http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java@175 PS11, Line 175: StmtRewriter.rewriteNonScalarSubqueries(operand, analyzer); > I like this suggestion, it does make things much easier and simple. Thanks for making these changes. I did not look at all failing tests, but I did into the following one: // Correlated predicates in the subquery's ON clause. Vary join column type. AnalyzesOk("select * from functional.alltypes t where t.id IN " + "(select a.id from functional.alltypesagg a left outer join " + "functional.alltypessmall s on s.int_col = t.int_col)"); The problem with this one is that the StmtRewriter relies on Subquery.isScalarSubquery(). The existing isScalarSubquery() relies on the type of the Subquery to determine whether a Subquery can be proven to be scalar at compile time. Since we have changed how the type is determined, we broke the isScalarSubquery() function. I hacked up the following to fix it: public boolean isScalarSubquery() { Preconditions.checkState(isAnalyzed_); if (!(this instanceof Subquery)) return false; Subquery subq = (Subquery) this; // Callers should be sure to analyze the limit clause of stmt SelectStmt stmt = (SelectStmt) subq.getStatement(); return stmt.returnsSingleRow() && getType().isScalarType(); } Of course, there are other ways to fix this, just wanted to point you in the right direction. Let me know if this needs further discussion. -- To view, visit http://gerrit.cloudera.org:8080/9005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0f52b93a60eeacedd242a2f17fa6b99c4fc38e06 Gerrit-Change-Number: 9005 Gerrit-PatchSet: 11 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 22 Mar 2018 23:08:32 +0000 Gerrit-HasComments: Yes
