Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Oracle support for backend database #588

Merged
merged 2 commits into from
Jan 27, 2025
Merged

Add Oracle support for backend database #588

merged 2 commits into from
Jan 27, 2025

Conversation

posulliv
Copy link
Contributor

Description

This PR does 2 things:

  1. update tests to use postgresql instead of h2
  2. adds Oracle as an option for the backend database. This required updates to the query history DAO since LIMIT is not supported in Oracle

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( X) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Jan 10, 2025
@posulliv posulliv requested a review from willmostly January 10, 2025 21:39
willmostly
willmostly requested changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few minor comments on the Oracle half. Will review the H2 ==> PG part asap

gateway-ha/pom.xml Show resolved Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/persistence/dao/QueryHistoryDao.java Outdated Show resolved Hide resolved
gateway-ha/src/test/java/io/trino/gateway/ha/persistence/TestDatabaseMigrationsOracle.java Outdated Show resolved Hide resolved
gateway-ha/src/test/java/io/trino/gateway/ha/persistence/TestDatabaseMigrationsOracle.java Outdated Show resolved Hide resolved
gateway-ha/src/test/java/io/trino/gateway/ha/persistence/TestDatabaseMigrationsOracle.java Outdated Show resolved Hide resolved
@posulliv posulliv requested a review from willmostly January 14, 2025 21:53
@posulliv
Copy link
Contributor Author

Thanks for the review! Updating based on comments.

willmostly
willmostly requested changes Jan 15, 2025
View reviewed changes
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Once this is documented it should be ready to ship

gateway-ha/src/test/java/io/trino/gateway/ha/TestGatewayHaSingleBackend.java Outdated Show resolved Hide resolved
gateway-ha/src/test/java/io/trino/gateway/ha/persistence/TestDatabaseMigrationsOracle.java Outdated Show resolved Hide resolved
@posulliv posulliv requested a review from ebyhr January 17, 2025 17:10
@posulliv
Copy link
Contributor Author

@ebyhr thank you for having a look! Updated based on your feedback.

@posulliv posulliv force-pushed the add-oracle-support branch 4 times, most recently from 44a0cfd to b64e2da Compare January 21, 2025 15:16
willmostly
willmostly requested changes Jan 22, 2025
View reviewed changes
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a little more cleanup and I think this should be good to go

docs/installation.md Outdated Show resolved Hide resolved
gateway-ha/src/test/java/io/trino/gateway/ha/HaGatewayTestUtils.java Outdated Show resolved Hide resolved
@posulliv posulliv requested a review from willmostly January 24, 2025 20:20
@posulliv
Copy link
Contributor Author

@willmostly thanks! updated again.

willmostly
willmostly approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@mosabua mosabua changed the title Add oracle support for backend database Add Oracle support for backend database Jan 24, 2025
@posulliv
Copy link
Contributor Author

thanks @ebyhr! pushed updates based on your comments.

@posulliv posulliv requested a review from ebyhr January 27, 2025 21:28
@ebyhr ebyhr merged commit bfe6c02 into trinodb:main Jan 27, 2025
2 checks passed
@github-actions github-actions bot added this to the 14 milestone Jan 27, 2025
@posulliv posulliv deleted the add-oracle-support branch January 28, 2025 01:30
@mosabua mosabua mentioned this pull request Jan 28, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willmostly willmostly willmostly approved these changes

@ebyhr ebyhr ebyhr approved these changes

@mosabua mosabua Awaiting requested review from mosabua

Assignees
No one assigned
Labels
cla-signed
Milestone
14
Development

Successfully merging this pull request may close these issues.
4 participants
@posulliv @mosabua @ebyhr @willmostly