[email protected] has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20506 )

Change subject: IMPALA-12465: Partial unicode column name support
......................................................................


Patch Set 3:

(5 comments)

> Patch Set 2:
>
> (1 comment)

http://gerrit.cloudera.org:8080/#/c/20506/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20506/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-12465: Partial unicode column name support
> nit: Don't need to call out that it's "Basic"
Done


http://gerrit.cloudera.org:8080/#/c/20506/2//COMMIT_MSG@7
PS2, Line 7: e suppo
> I don't think it's a bug fix, it's more like a new feature.
Done


http://gerrit.cloudera.org:8080/#/c/20506/2//COMMIT_MSG@9
PS2, Line 9: Impala depends on hive functions for column name
> nit: "primarily" implies there are other methods. Can you be more specific?
Done


http://gerrit.cloudera.org:8080/#/c/20506/2/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/20506/2/be/src/runtime/coordinator.cc@1172
PS2, Line 1172:       RPCErrorInfoPB rpc_error_info = 
aux_error.rpc_error_info();
> I'm confused why this is part of the patch. To avoid a conflict instead of
It's resolved now. It was due to a faulty rebase.


http://gerrit.cloudera.org:8080/#/c/20506/2/fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
File fe/src/main/java/org/apache/impala/analysis/ColumnDef.java:

http://gerrit.cloudera.org:8080/#/c/20506/2/fe/src/main/java/org/apache/impala/analysis/ColumnDef.java@207
PS2, Line 207:       throw new AnalysisException("Invalid column/field name: " 
+ colName_);
> Can you call out in the commit message the difference between MetaStoreUtil
I've updated the commit message, kindly let me know if it requires any further 
changes. We use validateName() for tables and databases, but we don't need it 
for column name validation.

I've updated the patch to accomodate USE_APACHE_HIVE, will test it as well and 
update.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ad9d63ac1b9631a0f4a433798bd5109aa2ed718
Gerrit-Change-Number: 20506
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Comment-Date: Fri, 29 Sep 2023 09:40:47 +0000
Gerrit-HasComments: Yes

Reply via email to