Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10571 )

Change subject: IMPALA-7106: Print rewritten SQL correctly when log trace is 
enabled
......................................................................


Patch Set 3:

(1 comment)

We currently have some stange behavior in the StmtRewriter. UnionStmt

http://gerrit.cloudera.org:8080/#/c/10571/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:

http://gerrit.cloudera.org:8080/#/c/10571/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@972
PS3, Line 972: .toSql()
- Why is plain toSql() called here, instead of carrying 'rewritten' like in 
UnionStmt?
- For SelectStmt, sqlString is strangely cleared in StmtRewriter: 
https://github.com/apache/impala/blob/4fda68a59d0a7f3855d13d5a4b682c7348abb0cb/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java#L100.
 So I think toRewrittenSql and toSql might print the same thing for a rewritten 
SelectStmt. I'm not sure if you want to clean this strange behavior up but it 
would be better adding some tests to document it at least.



--
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: 3
Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Jun 2018 20:31:53 +0000
Gerrit-HasComments: Yes

Reply via email to