-
Notifications
You must be signed in to change notification settings - Fork 378
change catalog default warehouse location to not use hive-style warehouse location #2059
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.
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
change catalog default warehouse location to not use hive-style warehouse location #2059
Conversation
could do something fancy with |
217d912
to
f393c38
Compare
.db
suffix in warehouse location.db
suffix in warehouse location for dynamo/hive/glue catalogs
@jayceslesar made it so aligns with the java implementation |
catalog=self, | ||
) | ||
|
||
def _get_default_warehouse_location(self, database_name: str, table_name: str) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we're repeating this a couple of times, should we move this into pyiceberg/catalog/__init__.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call, done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, I think this is a good change since we don't want to leak Hive legacy until eternity :)
.db
suffix in warehouse location for dynamo/hive/glue catalogs…ouse location (apache#2059) <!-- Thanks for opening a pull request! --> <!-- In the case this PR will resolve an issue, please replace ${GITHUB_ISSUE_ID} below with the actual Github issue id. --> <!-- Closes #${GITHUB_ISSUE_ID} --> Closes apache#2052 # Rationale for this change Aligns catalog behavior with the java reference implementation. [HiveCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java#L697-L736), [DynamoDbCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/aws/src/main/java/org/apache/iceberg/aws/dynamodb/DynamoDbCatalog.java#L185), and [GlueCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java#L291) all use `.db` suffix in warehouse location [JdbcCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java#L268), [HadoopCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java#L233-L245), and [InMemoryCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/core/src/main/java/org/apache/iceberg/inmemory/InMemoryCatalog.java#L106-L117) do not use `.db` suffix in warehouse location # Are these changes tested? Yes tests for sql catalog are modified to remove `.db` # Are there any user-facing changes? <!-- In the case of user-facing changes, please add the changelog label. -->
…ouse location (apache#2059) <!-- Thanks for opening a pull request! --> <!-- In the case this PR will resolve an issue, please replace ${GITHUB_ISSUE_ID} below with the actual Github issue id. --> <!-- Closes #${GITHUB_ISSUE_ID} --> Closes apache#2052 # Rationale for this change Aligns catalog behavior with the java reference implementation. [HiveCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java#L697-L736), [DynamoDbCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/aws/src/main/java/org/apache/iceberg/aws/dynamodb/DynamoDbCatalog.java#L185), and [GlueCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java#L291) all use `.db` suffix in warehouse location [JdbcCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java#L268), [HadoopCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java#L233-L245), and [InMemoryCatalog](https://github.com/apache/iceberg/blob/7537c3c3a2a6491abcf0c3ef58cc4d5dc6ac4bae/core/src/main/java/org/apache/iceberg/inmemory/InMemoryCatalog.java#L106-L117) do not use `.db` suffix in warehouse location # Are these changes tested? Yes tests for sql catalog are modified to remove `.db` # Are there any user-facing changes? <!-- In the case of user-facing changes, please add the changelog label. -->
Closes #2052
Rationale for this change
Aligns catalog behavior with the java reference implementation.
HiveCatalog, DynamoDbCatalog, and GlueCatalog all use
.db
suffix in warehouse locationJdbcCatalog, HadoopCatalog, and InMemoryCatalog do not use
.db
suffix in warehouse locationAre these changes tested?
Yes tests for sql catalog are modified to remove
.db
Are there any user-facing changes?