Alex Behm has posted comments on this change. Change subject: IMPALA-784: Use `-s in SHOW CREATE TABLE output ......................................................................
Patch Set 2: (2 comments) 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 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 Joiner.on(".").join(getStandaloneIdentSqlList((Splitter.on('.').split(ident)))); > 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 flavor. 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: IMPALA-2287 My basic question here is: Why did you add the splitting? -- 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: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Ivanfi <z...@cloudera.com> Gerrit-HasComments: Yes
