You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "adutra (via GitHub)" <gi...@apache.org> on 2025/06/03 15:33:25 UTC
[PR] Catalogs: consistently handle non-existent identifiers when loading tables/views [iceberg]
adutra opened a new pull request, #13223:
URL: https://github.com/apache/iceberg/pull/13223
Fixes #13115
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Catalogs: consistently handle non-existent identifiers when loading tables/views [iceberg]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #13223:
URL: https://github.com/apache/iceberg/pull/13223#discussion_r2130005166
##########
core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java:
##########
@@ -949,14 +951,37 @@ public void testLoadMetadataTable() {
assertThat(table.name()).isEqualTo(catalog.name() + "." + metaIdent);
}
- @Test
- public void testLoadMissingTable() {
+ @ParameterizedTest
+ @MethodSource("nonExistentTableIdentifiers")
+ public void testLoadMissingTable(TableIdentifier identifier) {
C catalog = catalog();
- assertThat(catalog.tableExists(TBL)).as("Table should not exist").isFalse();
- assertThatThrownBy(() -> catalog.loadTable(TBL))
+ assumeThat(supportsEmptyNamespace() || !identifier.namespace().isEmpty())
Review Comment:
I see what you mean, but I think it would introduce too many special cases: when confronted with invalid identifiers, some catalogs throw IAE in `tableExists` (BigQuery), some others throw IAE at `buildTable` (Hive), some others throw NSTE at `buildTable` (REST). It will be hard to come up with a single test for all catalogs.
And OTOH, if the catalog clearly doesn't support some specific configuration, its behavior under such configuration shouldn't be required to be normative, imo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Catalogs: consistently handle non-existent identifiers when loading tables/views [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13223:
URL: https://github.com/apache/iceberg/pull/13223#issuecomment-3066292820
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Catalogs: consistently handle non-existent identifiers when loading tables/views [iceberg]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on PR #13223:
URL: https://github.com/apache/iceberg/pull/13223#issuecomment-2945980917
@nastra I ported the relevant tests from #11963 and merged with the ones I had. Let me know if that looks better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Catalogs: consistently handle non-existent identifiers when loading tables/views [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13223:
URL: https://github.com/apache/iceberg/pull/13223#discussion_r2126083409
##########
core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java:
##########
@@ -949,14 +951,37 @@ public void testLoadMetadataTable() {
assertThat(table.name()).isEqualTo(catalog.name() + "." + metaIdent);
}
- @Test
- public void testLoadMissingTable() {
+ @ParameterizedTest
+ @MethodSource("nonExistentTableIdentifiers")
+ public void testLoadMissingTable(TableIdentifier identifier) {
C catalog = catalog();
- assertThat(catalog.tableExists(TBL)).as("Table should not exist").isFalse();
- assertThatThrownBy(() -> catalog.loadTable(TBL))
+ assumeThat(supportsEmptyNamespace() || !identifier.namespace().isEmpty())
Review Comment:
maybe just add a new test method, as otherwise this is effectively removing some test coverage from catalogs that don't support empty/nested namespaces
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Catalogs: consistently handle non-existent identifiers when loading tables/views [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #13223: Catalogs: consistently handle non-existent identifiers when loading tables/views
URL: https://github.com/apache/iceberg/pull/13223
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Catalogs: consistently handle non-existent identifiers when loading tables/views [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13223:
URL: https://github.com/apache/iceberg/pull/13223#issuecomment-3040476905
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Catalogs: consistently handle non-existent identifiers when loading tables/views [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #13223:
URL: https://github.com/apache/iceberg/pull/13223#issuecomment-2939275621
I think the issue is not just non-existing metadata identifiers but also the fact that you can't name a table like a metadata table name. @adutra could you please port over the tests from https://github.com/apache/iceberg/pull/11963 so that we can also fix https://github.com/apache/iceberg/issues/10550 with this PR?
I didn't have any capacity for https://github.com/apache/iceberg/pull/11963 but the approach there didn't work with nested namespaces.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org