Zoltan Borok-Nagy 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 23: (3 comments) http://gerrit.cloudera.org:8080/#/c/9005/23/fe/src/main/java/org/apache/impala/analysis/StatementBase.java File fe/src/main/java/org/apache/impala/analysis/StatementBase.java: http://gerrit.cloudera.org:8080/#/c/9005/23/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@43 PS23, Line 43: protected String origSqlString_ = null; > Thinking about this again, you right that your previous code set the origSq Done http://gerrit.cloudera.org:8080/#/c/9005/23/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java: http://gerrit.cloudera.org:8080/#/c/9005/23/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java@1174 PS23, Line 1174: AnalyzesOk("select id from functional.alltypestiny t where int_col < " + > My understanding is that we're not tackling correlated subqueries in this p I see, we'd need a SubPlan for the correct behavior, right? I extended StmtRewriter.validateCorrelatedSubquery() so this query is not allowed anymore. I also added a test with the query from my previous comment. http://gerrit.cloudera.org:8080/#/c/9005/23/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java@1395 PS23, Line 1395: AnalyzesOk("select count(1) from functional.alltypes " + > Regardless of what optimizations we apply to make the query fast, we must r Maybe we should think again the case we discussed in PS9 SingleNodePlanner.java, because PostgreSQL also raises error for that query. That could be fixed by adding an extra check in InlineViewRef to break the value transfer graph. I don't have a strong opinion about it, I just felt I should mention it. Fixing this case is not as easy, at least I coudn't figure out how to do it yet. In SingleNodePlanner.createScanNode() we assign all the predicates to the scan node that are bound by the tuple id of the TableRef. I played with Analyzer.evalAfterJoin(Expr) and the boolean variable 'evalAfterJoin' in Analyzer.getBoundPredicates(), but couldn't prevent the predicate push down. I also experimented with SingleNodePlanner.migrateConjunctsToInlineView(), but at that point I can't prevent predicate propagation as far as I can tell. There is also a comment about it in createInlineViewPlan(). The problematic predicate is created in StmtRewriter.createJoinConjunct(). Tried to save its tuple ids into Analyzer.globalState_, but later in the planner I see different ids, so I couldn't really use the stored ids. Do you have any tips? I'd prefer to fix it here rather than opening a jira for it. -- 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: 23 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: Wed, 18 Apr 2018 17:17:08 +0000 Gerrit-HasComments: Yes
