davseitsev commented on code in PR #12892:
URL: https://github.com/apache/iceberg/pull/12892#discussion_r2571889426
##########
core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java:
##########
@@ -192,6 +194,14 @@ protected boolean supportsEmptyNamespace() {
return false;
}
+ protected boolean supportsTableRename() {
Review Comment:
It’s related to how the base `CatalogTests` behave for catalogs that don't
support rename, like `TestBigQueryCatalog`.
Once I added my tests into `CatalogTests`, implementations that don't
support table rename started failing them. The `supportsTableRename()` flag is
the hook lets such catalogs opt out of the rename-related tests.
The base class wraps those tests in `assumeThat(supportsTableRename())`, and
[TestBigQueryCatalog](https://github.com/apache/iceberg/pull/12892/files/6265dfaef561d5db6bc0232364d8e447fffeaa3a#diff-d896e393063c5bbed20cd2248ec76c67efd8679420e3c5e440f6052d6d215c6e)
overrides `supportsTableRename()` to return `false` instead of overriding and
disabling each test with `@Disabled("BigQuery Metastore does not support rename
tables")`.
That's also consistent with the new `UNIQUE_TABLE_LOCATION` property being
defined as "only relevant for catalogs which support rename".
If you’d prefer to keep this PR strictly focused, I can drop
`supportsTableRename()` from `CatalogTests` for now and reintroduce the
explicit `@Disabled("BigQuery Metastore does not support rename tables")`
methods in `TestBigQueryCatalog`, and override+disable my tests in it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
