Zoltan Ivanfi 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))));
> I think we can still make progress here, I'm not sure I completely follow y
If I remember correctly, I encountered a problem with the 
com.cloudera.impala.analysis.TableName class. It stores the table name as a 
String, but the table name itself can be qualified in case of complex types, 
thus it can not be quoted by simply putting it between backticks.

It also seems that com.cloudera.impala.analysis.InsertStmt and 
com.cloudera.impala.catalog.View constructors take a List<String> parameter for 
columns and store it in the object.

It seems to me that in order to properly quote these, we would need to refactor 
the code to use a data structure capable of representing hierarchical names 
unambiguously to pass the table and column references around and to store them. 
I thought that the ongoing effort you mentioned is about addressing this.


-- 
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