Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/10571 )
Change subject: IMPALA-7106: Log the original and rewritten SQL when SQL rewrite fails ...................................................................... Patch Set 20: Code-Review+1 (6 comments) The patch looks reasonable to me in the current form (pending nits). http://gerrit.cloudera.org:8080/#/c/10571/18/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java File fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java: http://gerrit.cloudera.org:8080/#/c/10571/18/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@a100 PS18, Line 100: > There was an earlier discussion regarding this: https://gerrit.cloudera.org That makes sense as it is much closer to the user written SQL than before. http://gerrit.cloudera.org:8080/#/c/10571/20/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java: http://gerrit.cloudera.org:8080/#/c/10571/20/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@556 PS20, Line 556: if (t nit: merge http://gerrit.cloudera.org:8080/#/c/10571/20/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java File fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java: http://gerrit.cloudera.org:8080/#/c/10571/20/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java@81 PS20, Line 81: if (!rewritten) { : if (sqlString_ != null) return sqlString_; : } merge http://gerrit.cloudera.org:8080/#/c/10571/20/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/10571/20/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java@714 PS20, Line 714: WHERE a.int_col BETWEEN t.tinyint_col AND t.bigint_col) This makes more sense http://gerrit.cloudera.org:8080/#/c/10571/20/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java File fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java: http://gerrit.cloudera.org:8080/#/c/10571/20/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@253 PS20, Line 253: values(1, '2', 3, 4.1) I wasn't aware of this till now :-) http://gerrit.cloudera.org:8080/#/c/10571/20/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@261 PS20, Line 261: stmt = (StatementBase) AnalyzesOk("select * from (" + : "select * from functional.alltypes where id = (select 1 + 1)) a", ctx); : Assert.assertEquals( : "SELECT * FROM (SELECT * FROM functional.alltypes WHERE id = (SELECT 1 + 1)) a", : stmt.toSql()); : Assert.assertEquals( : "SELECT * FROM (SELECT * FROM functional.alltypes LEFT SEMI JOIN " + : "(SELECT 2) `$a$1` (`$c$1`) ON id = `$a$1`.`$c$1`) can we factor these out into a method? void foo(query, toSqlString, toSqlRewrittenString) { AnalyzesOk(query) AssertEquals(toSql(),toSqlString) AssertEquals(toSql(true),toSqlRewrittenstring) } -- To view, visit http://gerrit.cloudera.org:8080/10571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab58b0cc865135d261dd4a7f72be130f2e7bde53 Gerrit-Change-Number: 10571 Gerrit-PatchSet: 20 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Fri, 13 Jul 2018 20:39:59 +0000 Gerrit-HasComments: Yes
