You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org> on 2024/03/28 20:29:57 UTC

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21218


Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................

IMPALA-12925: Fix decimal data type for external JDBC table

Decimal type is a primitive data type for Impala. Current code returns
wrong values for columns with decimal data type in external JDBC tables.

This patch fixes wrong values returned from JDBC data source, and
supports pushing down decimal type of predicates to remote database
and remote Impala.

Testing:
 - Added Planner test for pushing down decimal type of predicates.
 - Added end-to-end unit-tests for decimal data type.
 - Passed core-tests.

Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
---
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M testdata/bin/create-ext-data-source-table.sql
M testdata/bin/load-ext-data-sources.sh
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test
M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test
10 files changed, 370 insertions(+), 8 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/2
-- 
To view, visit http://gerrit.cloudera.org:8080/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21218/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21218/5//COMMIT_MSG@16
PS5, Line 16: less than the decimal scales of the corresponding columns in the table
Is it just scale or precision also? I think we should enforce both.


http://gerrit.cloudera.org:8080/#/c/21218/5/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
File fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java:

http://gerrit.cloudera.org:8080/#/c/21218/5/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java@154
PS5, Line 154:             if (scalarType.scale < valScale) {
We should probably check both scale and precision here since Impala does enforce strict conversion from Decimal to Decimal.
https://impala.apache.org/docs/build/html/topics/impala_decimal.html

Updating test cases to capture this will also be good.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 Apr 2024 18:09:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21218/9/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
File fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java:

http://gerrit.cloudera.org:8080/#/c/21218/9/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java@194
PS9, Line 194:             new TStatus(TErrorCode.INTERNAL_ERROR, Lists.newArrayList(e.getMessage())));
Why is this an internal error? Isn't it misconfiguration between table definition and remote source?


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

http://gerrit.cloudera.org:8080/#/c/21218/9/testdata/bin/clean-mysql-env.sh@38
PS9, Line 38: rm -f /tmp/mysql_jdbc_decimal_tbl.*
Why are these created here instead of testdata/data?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 9
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 20:46:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 8: Code-Review+1

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21218/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21218/8//COMMIT_MSG@15
PS8, Line 15: The decimal precisions and scales of the columns in external JDBC table
nit: 
precisions -> precision
scales -> scale


http://gerrit.cloudera.org:8080/#/c/21218/8//COMMIT_MSG@16
PS8, Line 16: must be no less than the decimal precisions and scales of the
nit:
precisions -> precision
scales -> scale


http://gerrit.cloudera.org:8080/#/c/21218/8/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
File fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java:

http://gerrit.cloudera.org:8080/#/c/21218/8/fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java@155
PS8, Line 155:             // Check if there are enough precision and scale in the destination decimal.
nit: are -> is


http://gerrit.cloudera.org:8080/#/c/21218/8/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
File fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java:

http://gerrit.cloudera.org:8080/#/c/21218/8/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java@154
PS8, Line 154: * Decimal type of predicate data in TBinaryPredicate is saved as 'binary'.
             :          * Predicates are generated by Planner and sent to backend DataSourceScanNode.
             :          * When backend DataSourceScanNode calls ExternalDataSource.open() API, it sends
             :          * predicates to Java ExternalDataSourceExecutor object as TOpenParams.predicates
             :          * through JNI API. There is an issue to deserialize 'binary' type of data in
             :          * TBinaryPredicate when JniUtil.deserializeThrift() is called in function
             :          * ExternalDataSourceExecutor.open(). 'binary' type of data are changed to wrong
             :          * value. JniUtil.deserializeThrift() calls Thrift API TDeserializer.deserialize.
             :          * To work around this Thrift deserialization issue, string representation of the
             :          * decimal is returned by converting decimal to string with BigDecimal.toString()
             :          * Java API.
Maybe also file a JIRA for this incorrect conversion case when using JNI & thrift.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 8
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 14:31:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15778/ : 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/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 8
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 05:28:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 10:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 22:26:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15747/ : 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/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Apr 2024 06:13:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15751/ : 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/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Apr 2024 23:24:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

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/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................

IMPALA-12925: Fix decimal data type for external JDBC table

Decimal type is a primitive data type for Impala. Current code returns
wrong values for columns with decimal data type in external JDBC tables.

This patch fixes wrong values returned from JDBC data source, and
supports pushing down decimal type of predicates to remote database
and remote Impala.
The decimal precision and scale of the columns in external JDBC table
must be no less than the decimal precision and scale of the
corresponding columns in the table of remote database. Otherwise,
Impala fails with an error since it may cause truncation of decimal
data.

Testing:
 - Added Planner test for pushing down decimal type of predicates.
 - Added end-to-end unit-tests for tables with decimal type of columns
   for Postgres, MySQL, and Impala-to-Impala.
 - Passed core-tests.

Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Reviewed-on: http://gerrit.cloudera.org:8080/21218
Reviewed-by: Michael Smith <mi...@cloudera.com>
Reviewed-by: Abhishek Rawat <ar...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M testdata/bin/clean-mysql-env.sh
M testdata/bin/create-ext-data-source-table.sql
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test
M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test
M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test
14 files changed, 576 insertions(+), 21 deletions(-)

Approvals:
  Michael Smith: Looks good to me, but someone else must approve
  Abhishek Rawat: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 11
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

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

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................

IMPALA-12925: Fix decimal data type for external JDBC table

Decimal type is a primitive data type for Impala. Current code returns
wrong values for columns with decimal data type in external JDBC tables.

This patch fixes wrong values returned from JDBC data source, and
supports pushing down decimal type of predicates to remote database
and remote Impala.
The decimal precisions and scales of the columns in external JDBC table
must be no less than the decimal precisions and scales of the
corresponding columns in the table of remote database. Otherwise,
Impala fails with an error since it may cause truncation of decimal
data.

Testing:
 - Added Planner test for pushing down decimal type of predicates.
 - Added end-to-end unit-tests for tables with decimal type of columns
   for Postgres, MySQL, and Impala-to-Impala.
 - Passed core-tests.

Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
---
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M testdata/bin/clean-mysql-env.sh
M testdata/bin/create-ext-data-source-table.sql
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test
M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test
M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test
13 files changed, 566 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/8
-- 
To view, visit http://gerrit.cloudera.org:8080/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 8
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15724/ : 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/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Mar 2024 20:53:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21218/9/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
File fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java:

http://gerrit.cloudera.org:8080/#/c/21218/9/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java@194
PS9, Line 194:             new TStatus(TErrorCode.INTERNAL_ERROR, Lists.newArrayList(e.getMessage())));
> Why is this an internal error? Isn't it misconfiguration between table defi
The errors are caused by misconfiguration between JDBC table definition and remote tables in RDBMS, like upsupported column data type, invalid decimal precision and scale, etc. The error messages will be returned to client. We can define an error code for JDBC configuration.


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

http://gerrit.cloudera.org:8080/#/c/21218/9/testdata/bin/clean-mysql-env.sh@38
PS9, Line 38: rm -f /tmp/mysql_jdbc_decimal_tbl.*
> Why are these created here instead of testdata/data?
These files are created by testdata/bin/setup-mysql-env.sh when running unit-tests for MySQL. These are temp files used for creating table and loading data for MySQL when setting up MySQL server. We clean up these files after the MySQL server has been setup.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 9
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 21:09:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 10: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 22:24:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 10: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2024 09:16:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15776/ : 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/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 04:11:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

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

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................

IMPALA-12925: Fix decimal data type for external JDBC table

Decimal type is a primitive data type for Impala. Current code returns
wrong values for columns with decimal data type in external JDBC tables.

This patch fixes wrong values returned from JDBC data source, and
supports pushing down decimal type of predicates to remote database
and remote Impala.
The decimal precision and scale of the columns in external JDBC table
must be no less than the decimal precision and scale of the
corresponding columns in the table of remote database. Otherwise,
Impala fails with an error since it may cause truncation of decimal
data.

Testing:
 - Added Planner test for pushing down decimal type of predicates.
 - Added end-to-end unit-tests for tables with decimal type of columns
   for Postgres, MySQL, and Impala-to-Impala.
 - Passed core-tests.

Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
---
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M testdata/bin/clean-mysql-env.sh
M testdata/bin/create-ext-data-source-table.sql
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test
M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test
M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test
14 files changed, 576 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/10
-- 
To view, visit http://gerrit.cloudera.org:8080/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 10:

Thanks Abhishek and Michael.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 22:26:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 10:

Hit irrelevant IMPALA-11285. Try yo rerun


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2024 04:01:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................

IMPALA-12925: Fix decimal data type for external JDBC table

Decimal type is a primitive data type for Impala. Current code returns
wrong values for columns with decimal data type in external JDBC tables.

This patch fixes wrong values returned from JDBC data source, and
supports pushing down decimal type of predicates to remote database
and remote Impala.
The decimal scales of the columns in external JDBC table must be no
less than the decimal scales of the corresponding columns in the table
of remote database. Otherwise, exception will be thrown since it may
cause truncation of decimal data.

Testing:
 - Added Planner test for pushing down decimal type of predicates.
 - Added end-to-end unit-tests for tables with decimal type of columns
   for Postgres, MySQL, and Impala-to-Impala.
 - Passed core-tests.

Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
---
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M testdata/bin/clean-mysql-env.sh
M testdata/bin/create-ext-data-source-table.sql
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test
M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test
M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test
13 files changed, 524 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/5
-- 
To view, visit http://gerrit.cloudera.org:8080/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15750/ : 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/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Apr 2024 19:20:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................

IMPALA-12925: Fix decimal data type for external JDBC table

Decimal type is a primitive data type for Impala. Current code returns
wrong values for columns with decimal data type in external JDBC tables.

This patch fixes wrong values returned from JDBC data source, and
supports pushing down decimal type of predicates to remote database
and remote Impala.

Testing:
 - Added Planner test for pushing down decimal type of predicates.
 - Added end-to-end unit-tests for decimal data type.
 - Passed core-tests.

Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
---
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M testdata/bin/clean-mysql-env.sh
M testdata/bin/create-ext-data-source-table.sql
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test
M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test
M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test
13 files changed, 491 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/3
-- 
To view, visit http://gerrit.cloudera.org:8080/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 9:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21218/9/testdata/bin/clean-mysql-env.sh@38
PS9, Line 38: rm -f /tmp/mysql_jdbc_decimal_tbl.*
> These files are created by testdata/bin/setup-mysql-env.sh when running uni
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 9
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 21:11:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15781/ : 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/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 9
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 16:38:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 10:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 22:26:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 10: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2024 03:33:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

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

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................

IMPALA-12925: Fix decimal data type for external JDBC table

Decimal type is a primitive data type for Impala. Current code returns
wrong values for columns with decimal data type in external JDBC tables.

This patch fixes wrong values returned from JDBC data source, and
supports pushing down decimal type of predicates to remote database
and remote Impala.
The decimal precision and scale of the columns in external JDBC table
must be no less than the decimal precision and scale of the
corresponding columns in the table of remote database. Otherwise,
Impala fails with an error since it may cause truncation of decimal
data.

Testing:
 - Added Planner test for pushing down decimal type of predicates.
 - Added end-to-end unit-tests for tables with decimal type of columns
   for Postgres, MySQL, and Impala-to-Impala.
 - Passed core-tests.

Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
---
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M testdata/bin/clean-mysql-env.sh
M testdata/bin/create-ext-data-source-table.sql
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test
M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test
M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test
13 files changed, 568 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/9
-- 
To view, visit http://gerrit.cloudera.org:8080/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 9
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 10:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15782/ : 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/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2024 22:44:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 10:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2024 04:03:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

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

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................

IMPALA-12925: Fix decimal data type for external JDBC table

Decimal type is a primitive data type for Impala. Current code returns
wrong values for columns with decimal data type in external JDBC tables.

This patch fixes wrong values returned from JDBC data source, and
supports pushing down decimal type of predicates to remote database
and remote Impala.
The decimal precisions and scales of the columns in external JDBC table
must be no less than the decimal precisions and scales of the
corresponding columns in the table of remote database. Otherwise,
Impala fails with an error since it may cause truncation of decimal
data.

Testing:
 - Added Planner test for pushing down decimal type of predicates.
 - Added end-to-end unit-tests for tables with decimal type of columns
   for Postgres, MySQL, and Impala-to-Impala.
 - Passed core-tests.

Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
---
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M testdata/bin/clean-mysql-env.sh
M testdata/bin/create-ext-data-source-table.sql
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test
M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test
M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test
13 files changed, 565 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/7
-- 
To view, visit http://gerrit.cloudera.org:8080/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

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

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................

IMPALA-12925: Fix decimal data type for external JDBC table

Decimal type is a primitive data type for Impala. Current code returns
wrong values for columns with decimal data type in external JDBC tables.

This patch fixes wrong values returned from JDBC data source, and
supports pushing down decimal type of predicates to remote database
and remote Impala.
The decimal precisions and scales of the columns in external JDBC table
must be no less than the decimal precisions and scales of the
corresponding columns in the table of remote database. Otherwise,
Impala fails with an error since it may cause truncation of decimal
data.

Testing:
 - Added Planner test for pushing down decimal type of predicates.
 - Added end-to-end unit-tests for tables with decimal type of columns
   for Postgres, MySQL, and Impala-to-Impala.
 - Passed core-tests.

Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
---
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M testdata/bin/clean-mysql-env.sh
M testdata/bin/create-ext-data-source-table.sql
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test
M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test
M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test
13 files changed, 564 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/6
-- 
To view, visit http://gerrit.cloudera.org:8080/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21218 )

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15775/ : 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/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 Apr 2024 22:15:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12925: Fix decimal data type for external JDBC table

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

Change subject: IMPALA-12925: Fix decimal data type for external JDBC table
......................................................................

IMPALA-12925: Fix decimal data type for external JDBC table

Decimal type is a primitive data type for Impala. Current code returns
wrong values for columns with decimal data type in external JDBC tables.

This patch fixes wrong values returned from JDBC data source, and
supports pushing down decimal type of predicates to remote database
and remote Impala.
The decimal scales of the columns in external JDBC table must be no
less than the decimal scales of the corresponding columns in the table
of remote database. Otherwise, exception will be thrown since it may
cause truncation of decimal data.

Testing:
 - Added Planner test for pushing down decimal type of predicates.
 - Added end-to-end unit-tests for tables with decimal type of columns
   for Postgres, MySQL, and Impala-to-Impala.
 - Passed core-tests.

Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
---
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/util/QueryConditionUtil.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M testdata/bin/clean-mysql-env.sh
M testdata/bin/create-ext-data-source-table.sql
M testdata/bin/load-ext-data-sources.sh
M testdata/bin/setup-mysql-env.sh
M testdata/workloads/functional-planner/queries/PlannerTest/data-source-tables.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test
M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test
M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test
M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test
13 files changed, 490 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/21218/4
-- 
To view, visit http://gerrit.cloudera.org:8080/21218
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c9d2e0667c42c0e52436b158e3dfe3ec14b9e3b
Gerrit-Change-Number: 21218
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gs...@cloudera.com>