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 14:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/22134/14/fe/pom.xml
File fe/pom.xml:

http://gerrit.cloudera.org:8080/#/c/22134/14/fe/pom.xml@317
PS14, Line 317:       <optional>true</optional>
Why is this marked optional?


http://gerrit.cloudera.org:8080/#/c/22134/14/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
File 
fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java:

http://gerrit.cloudera.org:8080/#/c/22134/14/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@115
PS14, Line 115:           throw new JdbcDatabaseAccessException(
It's not polite to drop the original Exception, you can add it as an additional 
argument to chain them. Can we be any more specific than Exception? The only 
exception I see that needs to be handled is SqlException; the runtime 
exceptions from cacheMap_.put would all be due to incorrect usage.


http://gerrit.cloudera.org:8080/#/c/22134/14/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@135
PS14, Line 135:           throw new JdbcDatabaseAccessException(String.format(
Let's chain this Exception too.


http://gerrit.cloudera.org:8080/#/c/22134/14/testdata/bin/setup-mysql-env.sh
File testdata/bin/setup-mysql-env.sh:

http://gerrit.cloudera.org:8080/#/c/22134/14/testdata/bin/setup-mysql-env.sh@157
PS14, Line 157:   double_col      DOUBLE ,
nit: whitespace in "DOUBLE ,"



--
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: 14
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: Wed, 19 Feb 2025 19:32:09 +0000
Gerrit-HasComments: Yes

Reply via email to