Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22134 )

Change subject: IMPALA-12992: Support for Hive JDBC Storage handler tables
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/22134/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22134/4//COMMIT_MSG@11
PS4, Line 11: done by making JDBC table properties compatible with
Can you expand on how this is done? It looks like we translate when loading the 
table, and maintain that only in the Impala cluster, i.e. it's not written back 
to HMS.


http://gerrit.cloudera.org:8080/#/c/22134/4//COMMIT_MSG@14
PS4, Line 14: create-hive-jdbc-table.sql contains a sample example for
nit: "sample example" seems repetitive.


http://gerrit.cloudera.org:8080/#/c/22134/4/fe/src/main/java/org/apache/impala/catalog/TableLoader.java
File fe/src/main/java/org/apache/impala/catalog/TableLoader.java:

http://gerrit.cloudera.org:8080/#/c/22134/4/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@122
PS4, Line 122:       // Support for Hive JDBC storage handler
nit: can we extract this all to a function? It'd help to group it together and 
keep load() from getting too incomprehensible.


http://gerrit.cloudera.org:8080/#/c/22134/4/fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
File 
fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java:

http://gerrit.cloudera.org:8080/#/c/22134/4/fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java@35
PS4, Line 35:   JDBC_DRIVER_URL("driver.url", false),
Please address why this changed in the commit message.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 4
Gerrit-Owner: Pranav Lodha <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Pranav Lodha <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Fri, 17 Jan 2025 00:16:04 +0000
Gerrit-HasComments: Yes

Reply via email to