[Closed Duplicate] Add support for `TIMESTAMP` and `TIMESTAMP WITH TIME ZONE` types in Exasol connector #26255

skyglass · 2025-07-21T23:42:30Z

[Update]

Had to close this PR after confusion with separating commits.
Here is the new PR with separated commits and with all changes, based on the PR review comments:
#26259

Description

Added Exasol Trino connector support for Timestamp and Timestamp With Local Time Zone JDBC data types

Additional context and related issues

added correspondent tests for timestamp with precision data type
added correspondent tests for timestamp with local time zone with precision data type
implemented special addRoundTrip method with additional column expression parameter for the predicate to fix the test predicate assumption
all timestamp with timezone tests use input literal timestamp string, which is interpreted as a JVM timestamp string ("America/Bahia_Banderas") but expected literal strings are interpreted as UTC strings. Therefore the difference between these timestamps is 6 hours (with DST) or 5 (without DST); for some historical values, like 1970 for example, the difference can be 7, which is expected behaviour for timestamp with local time zone

Release notes

## Exasol Connect
* Add support for `timestamp` and `timestamp with time zone` types.

cla-bot · 2025-07-21T23:42:32Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

plugin/trino-exasol/src/main/java/io/trino/plugin/exasol/ExasolClient.java

plugin/trino-exasol/src/test/java/io/trino/plugin/exasol/TestExasolTimestampMapping.java

plugin/trino-exasol/src/main/java/io/trino/plugin/exasol/mapping/ColumnMapper.java

ebyhr

Please squash commits into one and follow https://trino.io/development/process.html#pull-request-and-commit-guidelines-

plugin/trino-exasol/src/test/java/io/trino/plugin/exasol/TestingExasolServer.java

cla-bot · 2025-07-22T01:33:23Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2025-07-22T01:40:48Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2025-07-22T01:46:36Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

ebyhr · 2025-07-22T01:51:18Z

@skyglass I recommend configuring the codestyle on your IDE first https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#code-style. Please feel free to ping me on Trino Slack if you don't know how to setup.

cla-bot · 2025-07-22T02:52:46Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

…l connector - implemented additional timestamp with precision data type for ExasolClient - added correspondent tests for timestamp with precision data type - started implementation of additional timestamp with local time zone data type for ExasolClient - updated exasol db version to 8.34.0 in integration tests - finished implementation of additional timestamp with local time zone data type for ExasolClient - implemented special addRoundTrip method with additional column expression parameter for the predicate to fix the test predicate assumption - all timestamp with timezone tests use input literal timestamp string, which is interpreted as a JVM timestamp string ("America/Bahia_Banderas") but expected literal strings are interpreted as UTC strings. Therefore the difference between these timestamps is 6 hours (with DST) or 5 (without DST); for some historical values, like 1970 for example, the difference can be 7, which is expected behaviour for timestamp with local time zone - implemented ColumnMapper, TimestampColumnMapper and TimestampWithTimeZoneColumnMapper to encapsulate mapping logic in separate classes and prevent ExasolClient from becoming too big - removed mapper classes and moved the logic to ExasolClient, based on the PR suggesions - removed unnecessary javadocs - improved javadocs and comments for testing timestamp with time zone by explaining why only jvm time zone is currently used for testing timestamp with time zone - removed using * import in ExasolClient based on the PR suggestions - removed TestExasolTimestampMapping and moved testing timestamp and timestamp with timezone to TestExasolTypeMapping, based on the PR suggestions - fixed checkstyle violations after enabling checkstyle plugin

cla-bot · 2025-07-22T03:05:39Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

plugin/trino-exasol/src/test/java/io/trino/plugin/exasol/TestExasolConnectorTest.java

plugin/trino-exasol/src/test/java/io/trino/plugin/exasol/TestExasolTypeMapping.java

plugin/trino-exasol/src/test/java/io/trino/plugin/exasol/TestingExasolServer.java

…l connector - implemented additional timestamp with precision data type for ExasolClient - added correspondent tests for timestamp with precision data type - started implementation of additional timestamp with local time zone data type for ExasolClient - updated exasol db version to 8.34.0 in integration tests - finished implementation of additional timestamp with local time zone data type for ExasolClient - implemented special addRoundTrip method with additional column expression parameter for the predicate to fix the test predicate assumption - all timestamp with timezone tests use input literal timestamp string, which is interpreted as a JVM timestamp string ("America/Bahia_Banderas") but expected literal strings are interpreted as UTC strings. Therefore the difference between these timestamps is 6 hours (with DST) or 5 (without DST); for some historical values, like 1970 for example, the difference can be 7, which is expected behaviour for timestamp with local time zone - implemented ColumnMapper, TimestampColumnMapper and TimestampWithTimeZoneColumnMapper to encapsulate mapping logic in separate classes and prevent ExasolClient from becoming too big - removed mapper classes and moved the logic to ExasolClient, based on the PR suggesions - removed unnecessary javadocs - improved javadocs and comments for testing timestamp with time zone by explaining why only jvm time zone is currently used for testing timestamp with time zone - removed using * import in ExasolClient based on the PR suggestions - removed TestExasolTimestampMapping and moved testing timestamp and timestamp with timezone to TestExasolTypeMapping, based on the PR suggestions - fixed checkstyle violations after enabling checkstyle plugin

- implemented additional timestamp with precision data type for ExasolClient - added correspondent tests for timestamp with precision data type - implemented additional timestamp with local time zone data type for ExasolClient - implemented special addRoundTrip method with additional column expression parameter for the predicate to fix the test predicate assumption - all timestamp with timezone tests use input literal timestamp string, which is interpreted as a JVM timestamp string ("America/Bahia_Banderas") but expected literal strings are interpreted as UTC strings. Therefore the difference between these timestamps is 6 hours (with DST) or 5 (without DST); for some historical values, like 1970 for example, the difference can be 7, which is expected behaviour for timestamp with local time zone - removed mapper classes and moved the logic to ExasolClient, based on the PR suggesions - removed unnecessary javadocs - improved javadocs and comments for testing timestamp with time zone by explaining why only jvm time zone is currently used for testing timestamp with time zone - removed using * import in ExasolClient based on the PR suggestions - removed TestExasolTimestampMapping and moved testing timestamp and timestamp with timezone to TestExasolTypeMapping, based on the PR suggestions

…s' into feature/724_additional_data_types # Conflicts: # plugin/trino-exasol/src/test/java/io/trino/plugin/exasol/TestExasolTypeMapping.java

cla-bot · 2025-07-22T09:22:16Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

…s' into feature/724_additional_data_types

cla-bot · 2025-07-22T09:36:10Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

skyglass · 2025-07-22T10:07:08Z

Sorry, had to close this PR after confusion with separating commits.
Here is the new PR with separated commits and with all changes, based on the PR review comments:
#26259

skyglass · 2025-07-23T15:31:55Z

Any updates on the new PR? #26259 ?

I really tried to fix the old PR, and I was able to fix the branch commit history, but the PR doesn't update the commit history anymore, after it was closed. And I don't have permissions to reopen it.

Maybe reopening this PR will force update the commit history. Or maybe, after some time, the cache will be invalidated and the PR will fetch the correct commit history.

But, in any case, the new PR is exactly the same, although it is sad that the original PR history would be lost. Sorry for the inconvenience.

github-actions bot added the exasol Exasol connector label Jul 21, 2025

ebyhr reviewed Jul 21, 2025

View reviewed changes

ebyhr changed the title ~~Feature/724 additional data types~~ Add support for TIMESTAMP and TIMESTAMP WITH TIME ZONE types in Exasol connector Jul 21, 2025

ebyhr reviewed Jul 22, 2025

View reviewed changes

plugin/trino-exasol/src/test/java/io/trino/plugin/exasol/TestingExasolServer.java Show resolved Hide resolved

skyglass force-pushed the feature/724_additional_data_types branch from 48a9bfc to ea52364 Compare July 22, 2025 03:05

skyglass changed the title ~~Add support for TIMESTAMP and TIMESTAMP WITH TIME ZONE types in Exasol connector~~ Add support for TIMESTAMP and TIMESTAMP WITH LOCAL TIME ZONE types in Exasol connector Jul 22, 2025

ebyhr changed the title ~~Add support for TIMESTAMP and TIMESTAMP WITH LOCAL TIME ZONE types in Exasol connector~~ Add support for TIMESTAMP and TIMESTAMP WITH TIME ZONE types in Exasol connector Jul 22, 2025

ebyhr added the needs-docs This pull request requires changes to the documentation label Jul 22, 2025

ebyhr reviewed Jul 22, 2025

View reviewed changes

skyglass added 3 commits July 22, 2025 11:20

Merge remote-tracking branch 'origin/feature/724_additional_data_type…

04a3e6c

…s' into feature/724_additional_data_types # Conflicts: # plugin/trino-exasol/src/test/java/io/trino/plugin/exasol/TestExasolTypeMapping.java

skyglass added 2 commits July 22, 2025 11:35

Update exasol db version to 8.34.0 in tests

68ec52e

Merge remote-tracking branch 'origin/feature/724_additional_data_type…

7756c66

…s' into feature/724_additional_data_types

skyglass closed this Jul 22, 2025

skyglass changed the title ~~Add support for TIMESTAMP and TIMESTAMP WITH TIME ZONE types in Exasol connector~~ [Closed Duplicate] Add support for TIMESTAMP and TIMESTAMP WITH TIME ZONE types in Exasol connector Jul 22, 2025

skyglass deleted the feature/724_additional_data_types branch July 22, 2025 10:14

[Closed Duplicate] Add support for TIMESTAMP and TIMESTAMP WITH TIME ZONE types in Exasol connector #26255

[Closed Duplicate] Add support for TIMESTAMP and TIMESTAMP WITH TIME ZONE types in Exasol connector #26255

Conversation

skyglass commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Additional context and related issues

Release notes

Uh oh!

cla-bot bot commented Jul 21, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ebyhr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cla-bot bot commented Jul 22, 2025

Uh oh!

cla-bot bot commented Jul 22, 2025

Uh oh!

cla-bot bot commented Jul 22, 2025

Uh oh!

ebyhr commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cla-bot bot commented Jul 22, 2025

Uh oh!

cla-bot bot commented Jul 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cla-bot bot commented Jul 22, 2025

Uh oh!

cla-bot bot commented Jul 22, 2025

Uh oh!

skyglass commented Jul 22, 2025

Uh oh!

skyglass commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

[Closed Duplicate] Add support for `TIMESTAMP` and `TIMESTAMP WITH TIME ZONE` types in Exasol connector #26255

[Closed Duplicate] Add support for `TIMESTAMP` and `TIMESTAMP WITH TIME ZONE` types in Exasol connector #26255

skyglass commented Jul 21, 2025 •

edited

Loading

ebyhr commented Jul 22, 2025 •

edited

Loading

skyglass commented Jul 23, 2025 •

edited

Loading