Doc: Add getting started with JDBC source #1470
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
singhpk234
requested review from
adutra,
ashvina,
dennishuo,
dimas-b,
eric-maynard,
jackye1995,
jbonofre,
vvcephei,
collado-mike and
snazy
as code owners
singhpk234
requested review from
RussellSpitzer,
takidau,
MonkeyCanCode,
flyrain and
ebyhr
as code owners
singhpk234
commented
Apr 27, 2025
singhpk234
commented
Apr 27, 2025
singhpk234
force-pushed
the
feature/getting-started-jdbc
branch
from
eff619c to
f711cc1
Compare
...n/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java
Outdated
Show resolved
Hide resolved
singhpk234
force-pushed
the
feature/getting-started-jdbc
branch
from
f6f8520 to
7d3f4ea
Compare
singhpk234
force-pushed
the
feature/getting-started-jdbc
branch
from
7d3f4ea to
712d1bd
Compare
flyrain
reviewed
Apr 28, 2025
There was a problem hiding this comment.
LGTM overall. It's a not blocker, but there'd be some other docs needed to be updated.
- https://polaris.apache.org/in-dev/unreleased/metastores/
- https://polaris.apache.org/in-dev/unreleased/configuring-polaris-for-production/#metastore-configuration, we will suggest JDBC over eclipseLink for production.
- https://polaris.apache.org/in-dev/unreleased/admin-tool/, this is blocked by the PR to reenable boostrap logic for jdbc.
adnanhemani
suggested changes
Apr 28, 2025
There was a problem hiding this comment.
Overall, looks good. Few comments below, and a few high-level comments:
- We also need to update the existing Quickstart documents to start using Relational JDBC instead of Eclipselink as well. @singhpk234 - did you want to make these changes? Or I can take that up after this merges.
- Have we tested all of these both locally and on at least one cloud provider? If not locally, that should be a blocker. For a cloud provider, it isn't a blocker - but we should take as a fast-follow to verify this.
adutra
reviewed
Apr 30, 2025
|
|
||
| This example requires `jq` to be installed on your machine. | ||
|
|
||
| 1. If such an image is not already present, build the Polaris image with support for EclipseLink and |
There was a problem hiding this comment.
Suggested change
| 1. If such an image is not already present, build the Polaris image with support for EclipseLink and | |
| 1. If such an image is not already present, build the Polaris image with support for JDBC persistence and |
| ``` | ||
| docker exec -it jdbc-trino-1 trino | ||
| ``` | ||
| Note, `trino-trino-1` is the name of the Docker container. |
There was a problem hiding this comment.
Suggested change
| Note, `trino-trino-1` is the name of the Docker container. | |
| Note, `jdbc-trino-1` is the name of the Docker container. |
| ``` | ||
|
|
||
| 6. Using Trino CLI: To access the Trino CLI, run this command: | ||
| ``` |
There was a problem hiding this comment.
Suggested change
| ``` | |
| ```shell |
| # under the License. | ||
| # | ||
|
|
||
| services: |
There was a problem hiding this comment.
Is it the same postgres container than for EclipseLink? Maybe we can avoid duplicating this?
| services: | ||
|
|
||
| polaris: | ||
| # IMPORTANT: the image MUST contain the Postgres JDBC driver and EclipseLink dependencies, see README for instructions |
There was a problem hiding this comment.
Suggested change
| # IMPORTANT: the image MUST contain the Postgres JDBC driver and EclipseLink dependencies, see README for instructions | |
| # IMPORTANT: the image MUST contain the Postgres JDBC driver and JDBC persistence, see README for instructions |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
About the change