You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Pranav Lodha (Code Review)" <ge...@cloudera.org> on 2024/12/13 08:21:13 UTC

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Pranav Lodha has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22134


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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M testdata/bin/load-ext-data-sources.sh
3 files changed, 97 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/1
-- 
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: newchange
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 1
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha 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 17:

(2 comments)

> Uploaded patch set 17.

http://gerrit.cloudera.org:8080/#/c/22134/16/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/16/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@134
PS16, Line 134:   
> nit: add two more spaces
Done


http://gerrit.cloudera.org:8080/#/c/22134/16/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@136
PS16, Line 136:           // createDataSource() in commons-dbcp 2.9 throws Exception.
> nit: I'd mention that we catch Exception because createDataSource in common
Done, its mentioned in https://issues.apache.org/jira/browse/IMPALA-13797



-- 
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: 17
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Feb 2025 04:27:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 10:

(3 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/22134/10/tests/custom_cluster/test_ext_data_sources.py
File tests/custom_cluster/test_ext_data_sources.py:

http://gerrit.cloudera.org:8080/#/c/22134/10/tests/custom_cluster/test_ext_data_sources.py@302
PS10, Line 302: #
flake8: E265 block comment should start with '# '


http://gerrit.cloudera.org:8080/#/c/22134/10/tests/custom_cluster/test_ext_data_sources.py@371
PS10, Line 371:  
flake8: W291 trailing whitespace


http://gerrit.cloudera.org:8080/#/c/22134/10/tests/custom_cluster/test_ext_data_sources.py@371
PS10, Line 371:         "hive.sql.dbcp.password.keystore" = 
line has trailing whitespace



-- 
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: 10
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Feb 2025 09:00:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 9:

(8 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py
File tests/custom_cluster/test_ext_data_sources.py:

http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py@191
PS9, Line 191: ,
flake8: E231 missing whitespace after ','


http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py@209
PS9, Line 209:     
flake8: W293 blank line contains whitespace


http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py@209
PS9, Line 209:     
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py@222
PS9, Line 222: .
flake8: E501 line too long (98 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py@236
PS9, Line 236: ,
flake8: E231 missing whitespace after ','


http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py@313
PS9, Line 313:     
flake8: W293 blank line contains whitespace


http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py@313
PS9, Line 313:     
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py@326
PS9, Line 326: .
flake8: E501 line too long (98 > 90 characters)



-- 
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: 9
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Feb 2025 17:33:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 11:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18235/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 11
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Feb 2025 09:44:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18227/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 9
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Feb 2025 18:18:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

'driver.url' is not mandatory now. Previously, the
'driver.url' field was mandatory in Impala for JDBC tables.
However, since Impala now supports Hive JDBC tables and
includes the necessary database JARs in the classpath,
this field is no longer required. This aligns Impala's
behavior with Hive, where 'driver.url' is not mandatory.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M tests/custom_cluster/test_ext_data_sources.py
10 files changed, 243 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/6
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 6
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
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 16: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22134/16/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/16/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@136
PS16, Line 136:           throw new JdbcDatabaseAccessException(
nit: I'd mention that we catch Exception because createDataSource in commons-dbcp 2.9 throws it, and mention the IMPALA ticket created to update commons-dbcp to a version that only throws SQLException.



-- 
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: 16
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2025 19:11:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18247/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Feb 2025 19:18:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#16). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

Impala includes JDBC drivers for PostgreSQL and MySQL
making 'driver.url' not mandatory in such cases. The
Impala JDBC driver is still required for Impala-to-Impala
JDBC connections. Additionally, Hive allows adding database
driver JARs at runtime via Beeline, enabling users to
dynamically include JDBC driver JARs. However, Impala does
not support adding database driver JARs at runtime,
making the driver.url field still useful
in cases where additional drivers are needed.

'hive.sql.query' property is not handled in this patch.
It'll be covered in a separate jira.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-mysql-tables.test
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test
M tests/custom_cluster/test_ext_data_sources.py
12 files changed, 804 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/16
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 16
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 18: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11517/


-- 
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: 18
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Feb 2025 22:50:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
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:

(1 comment)

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@107
PS14, Line 107:       if (Strings.isNullOrEmpty(driverUrl)) {
nit: there's a lot of duplication in the if and else blocks, but I also get why it's a bit tricky to clean up. Personally I'd spend some time trying to, but not going to hold up the review on it.



-- 
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Feb 2025 19:35:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala.

create-hive-jdbc-table.sql contains a sample example for
testing hive storage handler functionality.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
A testdata/bin/create-hive-jdbc-table.sql
M testdata/bin/load-ext-data-sources.sh
8 files changed, 133 insertions(+), 25 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/4
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 4
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Feb 2025 01:35:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

Impala includes JDBC drivers for PostgreSQL and MySQL
making 'driver.url' not mandatory in such cases. The
Impala JDBC driver is still required for Impala-to-Impala
JDBC connections. Additionally, Hive allows adding database
driver JARs at runtime via Beeline, enabling users to
dynamically include JDBC driver JARs. However, Impala does
not support adding database driver JARs at runtime,
making the driver.url field still useful
in cases where additional drivers are needed.

'hive.sql.query' property is not handled in this patch.
It'll be covered in a separate jira.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-mysql-tables.test
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test
M tests/custom_cluster/test_ext_data_sources.py
11 files changed, 380 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/11
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 11
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
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 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22134/11/testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test
File testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test:

http://gerrit.cloudera.org:8080/#/c/22134/11/testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test@20
PS11, Line 20: ====
please add more test cases here, like select some columns, select count(*), self join, out join with other table, etc.



-- 
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: 11
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Feb 2025 18:28:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
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 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22134/16/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/16/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@134
PS16, Line 134:   
nit: add two more spaces



-- 
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: 16
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2025 21:34:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:

(1 comment)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

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@265
PS8, Line 265: e
flake8: E722 do not use bare 'except'



-- 
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2025 11:34:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha 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 6:

> Patch Set 5:
> 
> Could you rebase to solve merge conflicts?

Done.


-- 
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: 6
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2025 08:29:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 19: Code-Review+2


-- 
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: 19
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Feb 2025 06:27:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Feb 2025 19:32:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 17:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18382/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 17
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Feb 2025 04:54:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha 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 15:

(4 comments)

> Uploaded patch set 15.

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@107
PS14, Line 107:       try {
> nit: there's a lot of duplication in the if and else blocks, but I also get
Ack


http://gerrit.cloudera.org:8080/#/c/22134/14/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@115
PS14, Line 115:           dbcpDs.setDriverClassLoader(driverLoader);
> It's not polite to drop the original Exception, you can add it as an additi
Done


http://gerrit.cloudera.org:8080/#/c/22134/14/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@135
PS14, Line 135:     Entry entry = null;
> Let's chain this Exception too.
Done


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 ,"
Done



-- 
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: 15
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Feb 2025 19:22:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 15:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/18351/ : Initial code review checks failed. See linked job for details on the failure.


-- 
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: 15
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Feb 2025 19:48:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18147/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2025 11:58:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
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 15:

(2 comments)

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@124
PS14, Line 124: throw new JdbcDatabaseAccessException(
              :               "Unexpected error while creating datasource.", e);
              :       }
keep the comments


http://gerrit.cloudera.org:8080/#/c/22134/15/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/15/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@117
PS15, Line 117:  null
set it as driverLocalPath



-- 
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: 15
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Feb 2025 22:48:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 18:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11517/ DRY_RUN=false


-- 
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: 18
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Feb 2025 17:23:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

Impala includes JDBC drivers for PostgreSQL and MySQL
making 'driver.url' not mandatory in such cases. The
Impala JDBC driver is still required for Impala-to-Impala
JDBC connections. Additionally, Hive allows adding database
driver JARs at runtime via Beeline, enabling users to
dynamically include JDBC driver JARs. However, Impala does
not support adding database driver JARs at runtime,
making the driver.url field still useful
in cases where additional drivers are needed.

'hive.sql.query' property is not handled in this patch.
It'll be covered in a separate jira.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-mysql-tables.test
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test
M tests/custom_cluster/test_ext_data_sources.py
11 files changed, 382 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/10
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 10
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22134/1/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/1/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@132
PS1, Line 132:         impala_tbl_props.put("driver.url", "/test-warehouse/data-sources/jdbc-drivers/postgresql-jdbc.jar");
line too long (108 > 90)



-- 
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: 1
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2024 08:22:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 16:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18367/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 16
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2025 18:07:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#15). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

Impala includes JDBC drivers for PostgreSQL and MySQL
making 'driver.url' not mandatory in such cases. The
Impala JDBC driver is still required for Impala-to-Impala
JDBC connections. Additionally, Hive allows adding database
driver JARs at runtime via Beeline, enabling users to
dynamically include JDBC driver JARs. However, Impala does
not support adding database driver JARs at runtime,
making the driver.url field still useful
in cases where additional drivers are needed.

'hive.sql.query' property is not handled in this patch.
It'll be covered in a separate jira.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-mysql-tables.test
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test
M tests/custom_cluster/test_ext_data_sources.py
12 files changed, 792 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/15
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 15
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#17). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

Impala includes JDBC drivers for PostgreSQL and MySQL
making 'driver.url' not mandatory in such cases. The
Impala JDBC driver is still required for Impala-to-Impala
JDBC connections. Additionally, Hive allows adding database
driver JARs at runtime via Beeline, enabling users to
dynamically include JDBC driver JARs. However, Impala does
not support adding database driver JARs at runtime,
making the driver.url field still useful
in cases where additional drivers are needed.

'hive.sql.query' property is not handled in this patch.
It'll be covered in a separate jira.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-mysql-tables.test
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test
M tests/custom_cluster/test_ext_data_sources.py
12 files changed, 805 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/17
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 17
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:

(7 comments)

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@130
PS4, Line 130:           throw new TableLoadingException("Required parameter: hive.sql.database.type is missing.");
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/22134/4/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@134
PS4, Line 134:         // impala_tbl_props.put("driver.url", "/test-warehouse/data-sources/jdbc-drivers/postgresql-jdbc.jar");
line too long (111 > 90)


http://gerrit.cloudera.org:8080/#/c/22134/4/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@137
PS4, Line 137:           throw new TableLoadingException("Required parameter: hive.sql.dbcp.password is missing.");
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/22134/4/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@143
PS4, Line 143:           throw new TableLoadingException("Required parameter: hive.sql.jdbc.url is missing.");
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/22134/4/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@149
PS4, Line 149:           throw new TableLoadingException("Required parameter: hive.sql.dbcp.username is missing.");
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/22134/4/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@155
PS4, Line 155:           throw new TableLoadingException("Required parameter: hive.sql.table is missing.");
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/22134/4/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@161
PS4, Line 161:           throw new TableLoadingException("Required parameter: hive.sql.jdbc.driver is missing.");
line too long (98 > 90)



-- 
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jan 2025 06:00:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

'driver.url' is not mandatory now. Previously, the
'driver.url' field was mandatory in Impala for JDBC tables.
However, since Impala now supports Hive JDBC tables and
includes the necessary database JARs in the classpath,
this field is no longer required. This aligns Impala's
behavior with Hive, where 'driver.url' is not mandatory.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M tests/custom_cluster/test_ext_data_sources.py
10 files changed, 248 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/8
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 8
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
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:

(1 comment)

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@179
PS8, Line 179: TestHiveJdbcTables
> Suggest to split the class as two classes for postgres and mysql respective
Or move MySQL test to existing class TestMySqlExtJdbcTables so that we don't need to download and set up MySQL docker twice.



-- 
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Feb 2025 02:15:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/17677/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 1
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2024 08:44:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha 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 1:

(1 comment)

> Patch Set 1:
> 
> > Patch Set 1: Verified-1
> > 
> > Build Failed 
> > 
> > https://jenkins.impala.io/job/gerrit-code-review-checks/11191/ : ABORTED
> 
> Uploading a new patch soon, please ignore this one.

http://gerrit.cloudera.org:8080/#/c/22134/1/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/1/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@132
PS1, Line 132: /postgresql-jdbc.jar
> Cannot hardcode driver path.
Hi, I think you mean hive-site.xml. I think we're not hardcoding the driver path.To run our jdbc tasks, we need to run copy-ext-data-sources.sh where 
 https://github.com/apache/impala/blob/2828e473710cc246dbdeb9e1da772c45881cfddb/testdata/bin/copy-ext-data-sources.sh#L44 path is defined. We're using the same and it will be same for all. All the other properties are extracted from the table properties stored in hms by hive. Note that this currently only works for postgresql.



-- 
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: 1
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 21 Dec 2024 15:39:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

Impala includes JDBC drivers for PostgreSQL and MySQL
making 'driver.url' not mandatory in such cases. The
Impala JDBC driver is still required for Impala-to-Impala
JDBC connections. Additionally, Hive allows adding database
driver JARs at runtime via Beeline, enabling users to
dynamically include JDBC driver JARs. However, Impala does
not support adding database driver JARs at runtime,
making the driver.url field still useful
in cases where additional drivers are needed.

'hive.sql.query' property is not handled in this patch.
It'll be covered in a separate jira.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-mysql-tables.test
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test
M tests/custom_cluster/test_ext_data_sources.py
11 files changed, 808 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/14
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 14
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 10:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18234/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 10
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Feb 2025 09:25:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
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 14: Code-Review+1

Looks good. Will give +2 after passing ozone/s3 tests.


-- 
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Feb 2025 18:58:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 19:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11524/ DRY_RUN=false


-- 
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: 19
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Feb 2025 06:27:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 6:

(9 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/22134/6/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/6/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@192
PS6, Line 192:   private Map<String, String> setHiveJdbcProperties(org.apache.hadoop.hive.metastore.api.Table msTbl)
line too long (101 > 90)


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py
File tests/custom_cluster/test_ext_data_sources.py:

http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@178
PS6, Line 178: class TestHiveJdbcTables(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@223
PS6, Line 223: v
flake8: U100 Unused argument 'vector'


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@241
PS6, Line 241:     
flake8: W293 blank line contains whitespace


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@241
PS6, Line 241:     
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@243
PS6, Line 243:     
flake8: W293 blank line contains whitespace


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@243
PS6, Line 243:     
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@262
PS6, Line 262: e
flake8: F841 local variable 'e' is assigned to but never used


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@264
PS6, Line 264: )
flake8: E501 line too long (91 > 90 characters)



-- 
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: 6
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2025 08:31:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 7:

(3 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/22134/7/tests/custom_cluster/test_ext_data_sources.py
File tests/custom_cluster/test_ext_data_sources.py:

http://gerrit.cloudera.org:8080/#/c/22134/7/tests/custom_cluster/test_ext_data_sources.py@178
PS7, Line 178: class TestHiveJdbcTables(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/22134/7/tests/custom_cluster/test_ext_data_sources.py@262
PS7, Line 262: e
flake8: F841 local variable 'e' is assigned to but never used


http://gerrit.cloudera.org:8080/#/c/22134/7/tests/custom_cluster/test_ext_data_sources.py@264
PS7, Line 264: )
flake8: E501 line too long (91 > 90 characters)



-- 
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: 7
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2025 10:02:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18146/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 7
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2025 10:25:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
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:

(1 comment)

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@179
PS8, Line 179: TestHiveJdbcTables
Suggest to split the class as two classes for postgres and mysql respectively. Setting up mysql docker is not stable, the test case will be skipped if failed to setup mysql docker. There is no such issue for postgres. We can run test for postgres in core and exhaustive mode, run test for mysql in exhaustive mode.



-- 
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Feb 2025 01:53:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/17898/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jan 2025 06:21:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 1: Verified-1

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/11191/ : ABORTED


-- 
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: 1
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2024 14:05:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha 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 1:

> Patch Set 1: Verified-1
> 
> Build Failed 
> 
> https://jenkins.impala.io/job/gerrit-code-review-checks/11191/ : ABORTED

Uploading a new patch soon, please ignore this one.


-- 
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: 1
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2024 14:09:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
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 18: Code-Review+2


-- 
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: 18
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Feb 2025 17:23:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
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 17: Code-Review+1


-- 
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: 17
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Feb 2025 04:50:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

Impala includes JDBC drivers for PostgreSQL and MySQL
making 'driver.url' not mandatory in such cases. The
Impala JDBC driver is still required for Impala-to-Impala
JDBC connections. Additionally, Hive allows adding database
driver JARs at runtime via Beeline, enabling users to
dynamically include JDBC driver JARs. However, Impala does
not support adding database driver JARs at runtime,
making the driver.url field still useful
in cases where additional drivers are needed.

'hive.sql.query' property is not handled in this patch.
It'll be covered in a separate jira.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-mysql-tables.test
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test
M tests/custom_cluster/test_ext_data_sources.py
11 files changed, 304 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/9
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 9
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 9:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/18205/ : Initial code review checks failed. See linked job for details on the failure.


-- 
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: 9
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Feb 2025 11:55:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
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 9:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/22134/9/testdata/bin/setup-mysql-env.sh@149
PS9, Line 149: id int,
             :   name varchar(20)
could you create a table with more data types of columns?


http://gerrit.cloudera.org:8080/#/c/22134/9/testdata/bin/setup-mysql-env.sh@153
PS9, Line 153: VALUES (1, 'India');
             : INSERT INTO country (id, name)
             : VALUES (2, 'Russia');
             : INSERT INTO country (id, name)
             : VALUES (3, 'USA');
insert three rows in one insert


http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py
File tests/custom_cluster/test_ext_data_sources.py:

http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py@231
PS9, Line 231: INVALIDATE METADATA {0}.country_postgres
also need to run IM for country_keystore_postgres table


http://gerrit.cloudera.org:8080/#/c/22134/9/tests/custom_cluster/test_ext_data_sources.py@335
PS9, Line 335: INVALIDATE METADATA {0}.country_mysql
also need to run IM for country_keystore_mysql table



-- 
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: 9
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Feb 2025 18:26:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha 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 11:

(3 comments)

> Uploaded patch set 11.

http://gerrit.cloudera.org:8080/#/c/22134/10/tests/custom_cluster/test_ext_data_sources.py
File tests/custom_cluster/test_ext_data_sources.py:

http://gerrit.cloudera.org:8080/#/c/22134/10/tests/custom_cluster/test_ext_data_sources.py@302
PS10, Line 302: i
> flake8: E265 block comment should start with '# '
Done


http://gerrit.cloudera.org:8080/#/c/22134/10/tests/custom_cluster/test_ext_data_sources.py@371
PS10, Line 371:         "hive.sql.dbcp.password.keystore" =
> line has trailing whitespace
Done


http://gerrit.cloudera.org:8080/#/c/22134/10/tests/custom_cluster/test_ext_data_sources.py@371
PS10, Line 371: 
> flake8: W291 trailing whitespace
Done



-- 
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: 11
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Feb 2025 09:17:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
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 <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Jan 2025 00:16:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 6:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/18145/ : Initial code review checks failed. See linked job for details on the failure.


-- 
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: 6
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2025 08:48:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

'driver.url' is not mandatory now. Previously, the
'driver.url' field was mandatory in Impala for JDBC tables.
However, since Impala now supports Hive JDBC tables and
includes the necessary database JARs in the classpath,
this field is no longer required. This aligns Impala's
behavior with Hive, where 'driver.url' is not mandatory.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M tests/custom_cluster/test_ext_data_sources.py
10 files changed, 244 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/22134/7
-- 
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: newpatchset
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 7
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Pranav Lodha (Code Review)" <ge...@cloudera.org>.
Pranav Lodha 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 6:

(5 comments)

> Patch Set 6:
> 
> (9 comments)
> 
> gerrit-auto-critic failed. You can reproduce it locally using command:
> 
>   python3 bin/jenkins/critique-gerrit-review.py --dryrun
> 
> To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py
File tests/custom_cluster/test_ext_data_sources.py:

http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@223
PS6, Line 223: v
> flake8: U100 Unused argument 'vector'
Done


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@241
PS6, Line 241:     
> flake8: W293 blank line contains whitespace
Done


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@243
PS6, Line 243:     
> flake8: W293 blank line contains whitespace
Done


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@262
PS6, Line 262: e
> flake8: F841 local variable 'e' is assigned to but never used
Done


http://gerrit.cloudera.org:8080/#/c/22134/6/tests/custom_cluster/test_ext_data_sources.py@264
PS6, Line 264: )
> flake8: E501 line too long (91 > 90 characters)
Done



-- 
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: 6
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2025 09:58:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22134 )

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

IMPALA-12992: Support for Hive JDBC Storage handler tables

This is an enhancement request to support JDBC tables
created by Hive JDBC Storage handler. This is essentially
done by making JDBC table properties compatible with
Impala. It is done by translating when loading the table,
and maintaining that only in the Impala cluster, i.e. it's
not written back to HMS.

Impala includes JDBC drivers for PostgreSQL and MySQL
making 'driver.url' not mandatory in such cases. The
Impala JDBC driver is still required for Impala-to-Impala
JDBC connections. Additionally, Hive allows adding database
driver JARs at runtime via Beeline, enabling users to
dynamically include JDBC driver JARs. However, Impala does
not support adding database driver JARs at runtime,
making the driver.url field still useful
in cases where additional drivers are needed.

'hive.sql.query' property is not handled in this patch.
It'll be covered in a separate jira.

Testing: End-to-end tests are included in
test_ext_data_sources.py.

Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Reviewed-on: http://gerrit.cloudera.org:8080/22134
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfig.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-mysql-tables.test
A testdata/workloads/functional-query/queries/QueryTest/hive-jdbc-postgres-tables.test
M tests/custom_cluster/test_ext_data_sources.py
12 files changed, 805 insertions(+), 23 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
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: merged
Gerrit-Change-Id: I1674b93a02f43df8c1a449cdc54053cc80d9c458
Gerrit-Change-Number: 22134
Gerrit-PatchSet: 20
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 19: Verified+1


-- 
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: 19
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Feb 2025 11:44:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12992: Support for Hive JDBC Storage handler tables

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
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 15:

(2 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:     </dependency>
> Why is this marked optional?
Done


http://gerrit.cloudera.org:8080/#/c/22134/15/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/15/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java@125
PS15, Line 125:               "Unexpected error while creating datasource.", e);
I don't think this is quite unexpected. copyFileFromUriToLocal can throw IOException, and I think URLClassLoader may also throw some exceptions. I think handling each exception type with a relevant message makes sense.



-- 
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: 15
Gerrit-Owner: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Pranav Lodha <pr...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Feb 2025 19:43:04 +0000
Gerrit-HasComments: Yes