Alex Behm has posted comments on this change.

Change subject: IMPALA-784: Use `-s in SHOW CREATE TABLE output
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4527/2/fe/src/main/java/com/cloudera/impala/analysis/ToSqlUtils.java
File fe/src/main/java/com/cloudera/impala/analysis/ToSqlUtils.java:

Line 76:     return 
Joiner.on(".").join(getStandaloneIdentSqlList((Splitter.on('.').split(ident))));
> If I remember correctly, I encountered a problem with the com.cloudera.impa
1. It would be good talk about a concrete example. From my point of view, I 
don't see how such a TableName would ever be constructed. For nested types, we 
usually avoid TableName and prefer Path instead.

2. I don't think your comment on InsertStmt and View is relevant here. 
InsertStmt takes a list of columns which must be quoted without splitting. The 
same goes for the column labels taken in View. So I don't see a problem here.

3. I agree there may be some required work to properly quote nested types in 
some case. It would be good understand what these case are exactly before 
abandoning the effort.

Happy to discuss over a meeting / gchat if you think that would be helpful.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e66439daa35d2e3725e7277c16f3cc5fdb6f128
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <z...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <z...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to