RussellSpitzer commented on code in PR #15614:
URL: https://github.com/apache/iceberg/pull/15614#discussion_r2957129906
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -375,6 +383,26 @@ public boolean purgeTable(Identifier ident) {
String metadataFileLocation =
((HasTableOperations)
table).operations().current().metadataFileLocation();
+ boolean isRestCatalog =
+ this.icebergCatalog instanceof RESTCatalog
+ || this.icebergCatalog instanceof RESTSessionCatalog
+ || (this.icebergCatalog instanceof CachingCatalog
+ && ((CachingCatalog)
this.icebergCatalog).wrapped_is_instance(RESTCatalog.class));
+
+ if (isRestCatalog && this.restCatalogPurge) {
+ // Delegate purge to REST catalog - allows server-side features like
UNDROP
+ return dropTableWithPurging(ident);
Review Comment:
I kind of think these helpers
dropTableWithPurging and dropTableWithoutPurging
are a little confusing since we actually purge the table in both cases (just
not within those specific methods)
I would recommend either
1. have a single helper catalogDropTable(ident, purge)
2. Keep two methods with but give them catalog names catalogDropOnly and
catalogDropAndPurge
Trying to separate the idea of what is happening in the function from the
whole code path
--
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]
