Wenzhe Zhou 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 8: (12 comments) http://gerrit.cloudera.org:8080/#/c/22134/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22134/8//COMMIT_MSG@19 PS8, Line 19: database JARs database driver JARs http://gerrit.cloudera.org:8080/#/c/22134/8//COMMIT_MSG@20 PS8, Line 20: his field is no longer required Impala only include jdbc drivers of Postgres and MySql. The Impala jdbc driver is still required for impala-impala jdbc connection. The other difference is that Hive allow to jars in runtime from beeline so that user can add driver jars in runtime. But Impala don't allow to add jars in run time. So driver.url is still useful. http://gerrit.cloudera.org:8080/#/c/22134/8/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java File fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java: http://gerrit.cloudera.org:8080/#/c/22134/8/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java@248 PS8, Line 248: } unnecessary change http://gerrit.cloudera.org:8080/#/c/22134/8/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/8/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@193 PS8, Line 193: ( left parenthesis should be moved to last line http://gerrit.cloudera.org:8080/#/c/22134/8/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@204 PS8, Line 204: hive.sql.dbcp.password password could be null. Also need to handle the case hive.sql.dbcp.password.keystore and hive.sql.dbcp.password.key are used, instead of hive.sql.dbcp.password" http://gerrit.cloudera.org:8080/#/c/22134/8/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@225 PS8, Line 225: hive.sql.table either hive.sql.table or hive.sql.query is set. http://gerrit.cloudera.org:8080/#/c/22134/8/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@268 PS8, Line 268: } unnecessary change http://gerrit.cloudera.org:8080/#/c/22134/8/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/8/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@107 PS8, Line 107: driverUrl == null || driverUrl.isEmpty() call Strings.isNullOrEmpty(driverUrl) http://gerrit.cloudera.org:8080/#/c/22134/8/tests/custom_cluster/test_ext_data_sources.py File tests/custom_cluster/test_ext_data_sources.py: http://gerrit.cloudera.org:8080/#/c/22134/8/tests/custom_cluster/test_ext_data_sources.py@245 PS8, Line 245: hdfs:///test-warehouse this might not work for ozone and s3 builds. please run ozone and s3 builds with https://master-03.jenkins.cloudera.com/job/impala-private-parameterized/ http://gerrit.cloudera.org:8080/#/c/22134/8/tests/custom_cluster/test_ext_data_sources.py@278 PS8, Line 278: verify select results http://gerrit.cloudera.org:8080/#/c/22134/8/tests/custom_cluster/test_ext_data_sources.py@279 PS8, Line 279: postgres mysql http://gerrit.cloudera.org:8080/#/c/22134/8/tests/custom_cluster/test_ext_data_sources.py@281 PS8, Line 281: please add case to use keystore -- 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: 8 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: Sat, 01 Feb 2025 01:35:39 +0000 Gerrit-HasComments: Yes
