impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5286: Query fails due to Kudu column name case
Date Wed, 17 May 2017 20:13:23 GMT
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5286: Query fails due to Kudu column name case
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6902/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

Line 946:     String key = slotPath.toString().toLowerCase();
> This works just fine for other tables. The slotPath and names in Columns ar
Its true that we don't convert Kudu column names to lower case when creating the Impala KuduColumn
objects while loading the table.

We could in theory eliminate this bug by doing that, but that would introduce the problem
that we couldn't locate the columns with org.apache.kudu.Schema.getColumn, eg in KuduScanNode.
Unless we want to ask Kudu to supply a Schema.getColumnCaseInsensitive (which I doubt they
would like, since they allow columns with names that only differ by casing), we have to preserve
the original casing somewhere.

A possible option would be to provide a KuduColumn.getNameWithOriginalCase (or something better
named) and then general code like this can rely on getName as it does for other table types
and only Kudu specific code would need to call getNameWithOriginalCase.

That seems a little brittle, though, if people in the future writing Kudu specific code don't
realize that they have to call the special function. Thoughts?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I14aba88510012174716691b9946e1c7d54d01b44
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message