Skip to content

Core: Add property to disable table initialization for JdbcCatalog#10124

Merged
nastra merged 1 commit into
apache:mainfrom
mrcnc:add-initialize-tables-property
Apr 29, 2024
Merged

Core: Add property to disable table initialization for JdbcCatalog#10124
nastra merged 1 commit into
apache:mainfrom
mrcnc:add-initialize-tables-property

Conversation

@mrcnc

@mrcnc mrcnc commented Apr 11, 2024

Copy link
Copy Markdown
Contributor

It would be helpful to disable the creation of the catalog tables used by the JdbcCatalog to maintain the principle of least privilege. In my scenario, I would like to perform database migrations with a migration tool using a user with DDL privileges and use a separate user with only DML privileges for connecting the JdbcCatalog to the database.

@github-actions github-actions Bot added the core label Apr 11, 2024
Comment thread core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java Outdated
@mrcnc mrcnc force-pushed the add-initialize-tables-property branch 3 times, most recently from 6a40f22 to 0115a15 Compare April 11, 2024 18:57
Comment thread core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java
@mrcnc mrcnc force-pushed the add-initialize-tables-property branch from 0115a15 to 6bb9fb7 Compare April 12, 2024 15:27
@mrcnc

mrcnc commented Apr 12, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for the review and suggestions @nastra! I believe I've addressed all the feedback in the latest commit

@nastra

nastra commented Apr 13, 2024

Copy link
Copy Markdown
Contributor

thanks @mrcnc, I'll take another look on monday. @jbonofre could you also take a look please?

@jbonofre jbonofre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM to me. Thanks !

@jbonofre

Copy link
Copy Markdown
Member

This PR is ok for me. I will prepare another pr to provide create statement as sql file and document it.

Comment thread core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java Outdated

@nastra nastra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, but would be good to address the nits

@mrcnc mrcnc force-pushed the add-initialize-tables-property branch from 6bb9fb7 to 2d096e8 Compare April 15, 2024 13:23
@nastra

nastra commented Apr 16, 2024

Copy link
Copy Markdown
Contributor

@mrcnc looks like there's a merge conflict. Can you rebase please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants