Alex Behm has posted comments on this change.

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

Patch Set 2:

File fe/src/main/java/com/cloudera/impala/analysis/

Line 69:   public static String getIdentSql(String ident) {
> I agree that it's slighly less pleasing to the eye but I think correctness/
For SHOW CREATE TABLE and CREATE VIEW it's fine to quote all identifiers, but I 
don't think we should needlessly pollute error messages, e.g., in 
AnalysisExceptions. To the best of my knowledge, Hive does not quote 
identifiers in error reports, and neither should we.

Line 76:     return 
> Hmmm. Interesting. On the other hand:
Those are two separate issues. In your example, we are guarding against a 
limitation in the Hive Metastore that disallows certain column names. 
Generally, the Hive Metastore is pretty restrictive.

However, within an Impala query you can use more or less arbitrary identifiers, 
for better of for worse. There's an argument to be made that we should not 
allow that, but that's the state of the world today.

Ambiguities are a reality that we cannot escape, even if we disallowed dots in 
quoted identifiers. If you are curious, you can look at 
AnalyzeStmts#TestSlotRefPathAmbiguity() and the other *Ambiguity tests to get a 

I still don't see how the specific problem we are discussing is not solvable. 
This function should accept a single identifier and quote it. It's up to the 
caller (who has more knowledge about the identifier) to pass in the right value.

For the struct case there is an existing JIRA to change the behavior: 

My basic question here is: Why did you add the splitting?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e66439daa35d2e3725e7277c16f3cc5fdb6f128
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Reviewer: Zoltan Ivanfi <>
Gerrit-HasComments: Yes

Reply via email to