You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "iprithv (via GitHub)" <gi...@apache.org> on 2026/09/18 01:58:10 UTC
[PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
iprithv opened a new pull request, #5544:
URL: https://github.com/apache/polaris/pull/5544
Polaris DROP is hard from the catalog's point of view: the active name disappears immediately, and there is no hold period. Operators who need a recoverable drop (compliance / legal hold, Nessie-to-Polaris migration) have no first-class path.
part of #5054.
This is **Phase 1 only**. Default remains today's hard DROP. The GitHub RFC (R4) freed the identifier on soft-delete; this PR **reserves the name during the hold**, matching the review comment that name reuse should wait until unique table locations are the catalog invariant. Drop-and-recreate ETL is unchanged on catalogs that leave the feature off. Catalogs that enable it can `DROP PURGE` when they want the name back immediately.
current behavior:
`DROP TABLE` without purge permanently removes the catalog entity. Recreate of the same identifier succeeds. There is no hold, and Iceberg REST list/load/HEAD cannot see a dropped table because it is gone.
expected behavior (opt-in):
When `polaris.config.table-soft-delete.enabled=true` on an INTERNAL catalog:
| Action | Result |
|---|---|
| `DROP TABLE` without purge | Soft-delete: entity retained, metadata location retained, files not deleted |
| Iceberg REST list / load / HEAD | Hidden (`isDropped()`) |
| `CREATE` / register same identifier during hold | Conflict (name reserved) |
| `DROP TABLE PURGE` | Immediate permanent delete (unchanged) |
| After hold (`P7D` default) | Next list/create/drop-namespace in that namespace permanently removes catalog state |
File cleanup on expire is off unless `polaris.config.table-soft-delete.purge-data-on-permanent-delete=true`. Enabling the flag on an external catalog returns 400.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5544:
URL: https://github.com/apache/polaris/pull/5544#discussion_r4051696521
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -799,6 +779,44 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(10)
.buildFeatureConfiguration();
+ public static final FeatureConfiguration<Boolean> TABLE_SOFT_DELETE_ENABLED =
+ PolarisConfiguration.<Boolean>builder()
+ .key("TABLE_SOFT_DELETE_ENABLED")
+ .catalogConfig("polaris.config.table-soft-delete.enabled")
+ .description(
+ "If set to true, DROP TABLE without purge marks an INTERNAL Iceberg table as"
+ + " soft-deleted for a hold period instead of permanently removing it. Soft-deleted"
+ + " tables are hidden from Iceberg REST list/load/HEAD, the identifier stays"
+ + " reserved until permanent delete, and data files are not deleted. Default is"
+ + " false (today's hard DROP). DROP TABLE with purge remains an immediate"
Review Comment:
nit: `today` is different every day 😉 Maybe just `hard DROP`?
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -799,6 +779,44 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(10)
.buildFeatureConfiguration();
+ public static final FeatureConfiguration<Boolean> TABLE_SOFT_DELETE_ENABLED =
+ PolarisConfiguration.<Boolean>builder()
+ .key("TABLE_SOFT_DELETE_ENABLED")
+ .catalogConfig("polaris.config.table-soft-delete.enabled")
+ .description(
+ "If set to true, DROP TABLE without purge marks an INTERNAL Iceberg table as"
+ + " soft-deleted for a hold period instead of permanently removing it. Soft-deleted"
+ + " tables are hidden from Iceberg REST list/load/HEAD, the identifier stays"
+ + " reserved until permanent delete, and data files are not deleted. Default is"
+ + " false (today's hard DROP). DROP TABLE with purge remains an immediate"
+ + " permanent delete.")
+ .defaultValue(false)
+ .buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<String> TABLE_SOFT_DELETE_HOLD_PERIOD =
+ PolarisConfiguration.<String>builder()
+ .key("TABLE_SOFT_DELETE_HOLD_PERIOD")
Review Comment:
How do you envision actually purging soft-deleted tables?
Cf. https://lists.apache.org/thread/kqm0w38p7bnojq455yz7d2vdfp6ky1h7
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -799,6 +779,44 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(10)
.buildFeatureConfiguration();
+ public static final FeatureConfiguration<Boolean> TABLE_SOFT_DELETE_ENABLED =
+ PolarisConfiguration.<Boolean>builder()
+ .key("TABLE_SOFT_DELETE_ENABLED")
+ .catalogConfig("polaris.config.table-soft-delete.enabled")
+ .description(
+ "If set to true, DROP TABLE without purge marks an INTERNAL Iceberg table as"
+ + " soft-deleted for a hold period instead of permanently removing it. Soft-deleted"
+ + " tables are hidden from Iceberg REST list/load/HEAD, the identifier stays"
+ + " reserved until permanent delete, and data files are not deleted. Default is"
+ + " false (today's hard DROP). DROP TABLE with purge remains an immediate"
+ + " permanent delete.")
+ .defaultValue(false)
+ .buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<String> TABLE_SOFT_DELETE_HOLD_PERIOD =
+ PolarisConfiguration.<String>builder()
+ .key("TABLE_SOFT_DELETE_HOLD_PERIOD")
+ .catalogConfig("polaris.config.table-soft-delete.hold-period")
+ .description(
+ "ISO-8601 duration to retain a soft-deleted Iceberg table before it becomes eligible"
+ + " for permanent delete. Used only when TABLE_SOFT_DELETE_ENABLED is true."
+ + " Default is P7D (seven days).")
+ .defaultValue("P7D")
+ .buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<Boolean>
+ TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE =
+ PolarisConfiguration.<Boolean>builder()
+ .key("TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE")
+ .catalogConfig("polaris.config.table-soft-delete.purge-data-on-permanent-delete")
+ .description(
+ "If set to true, permanent delete of a previously soft-deleted Iceberg table"
+ + " enqueues the existing entity-cleanup task so data and metadata files may"
+ + " be removed. Used only when TABLE_SOFT_DELETE_ENABLED is true. Default is"
+ + " false (catalog state is removed; files are left in place).")
Review Comment:
If `false` reusing the table name will effectively enable the new table owner to access old table's data. This is still a serious risk, I think... until Polaris enforces unique table locations.
I'd prefer to avoid this risk by always deleting data files on purge.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
Posted by "iprithv (via GitHub)" <gi...@apache.org>.
iprithv commented on code in PR #5544:
URL: https://github.com/apache/polaris/pull/5544#discussion_r4052678347
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -799,6 +779,44 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(10)
.buildFeatureConfiguration();
+ public static final FeatureConfiguration<Boolean> TABLE_SOFT_DELETE_ENABLED =
+ PolarisConfiguration.<Boolean>builder()
+ .key("TABLE_SOFT_DELETE_ENABLED")
+ .catalogConfig("polaris.config.table-soft-delete.enabled")
+ .description(
+ "If set to true, DROP TABLE without purge marks an INTERNAL Iceberg table as"
+ + " soft-deleted for a hold period instead of permanently removing it. Soft-deleted"
+ + " tables are hidden from Iceberg REST list/load/HEAD, the identifier stays"
+ + " reserved until permanent delete, and data files are not deleted. Default is"
+ + " false (today's hard DROP). DROP TABLE with purge remains an immediate"
Review Comment:
updated to `hard DROP` :)
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
Posted by "iprithv (via GitHub)" <gi...@apache.org>.
iprithv commented on code in PR #5544:
URL: https://github.com/apache/polaris/pull/5544#discussion_r4052676631
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -799,6 +779,44 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(10)
.buildFeatureConfiguration();
+ public static final FeatureConfiguration<Boolean> TABLE_SOFT_DELETE_ENABLED =
+ PolarisConfiguration.<Boolean>builder()
+ .key("TABLE_SOFT_DELETE_ENABLED")
+ .catalogConfig("polaris.config.table-soft-delete.enabled")
+ .description(
+ "If set to true, DROP TABLE without purge marks an INTERNAL Iceberg table as"
+ + " soft-deleted for a hold period instead of permanently removing it. Soft-deleted"
+ + " tables are hidden from Iceberg REST list/load/HEAD, the identifier stays"
+ + " reserved until permanent delete, and data files are not deleted. Default is"
+ + " false (today's hard DROP). DROP TABLE with purge remains an immediate"
+ + " permanent delete.")
+ .defaultValue(false)
+ .buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<String> TABLE_SOFT_DELETE_HOLD_PERIOD =
+ PolarisConfiguration.<String>builder()
+ .key("TABLE_SOFT_DELETE_HOLD_PERIOD")
+ .catalogConfig("polaris.config.table-soft-delete.hold-period")
+ .description(
+ "ISO-8601 duration to retain a soft-deleted Iceberg table before it becomes eligible"
+ + " for permanent delete. Used only when TABLE_SOFT_DELETE_ENABLED is true."
+ + " Default is P7D (seven days).")
+ .defaultValue("P7D")
+ .buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<Boolean>
+ TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE =
+ PolarisConfiguration.<Boolean>builder()
+ .key("TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE")
+ .catalogConfig("polaris.config.table-soft-delete.purge-data-on-permanent-delete")
+ .description(
+ "If set to true, permanent delete of a previously soft-deleted Iceberg table"
+ + " enqueues the existing entity-cleanup task so data and metadata files may"
+ + " be removed. Used only when TABLE_SOFT_DELETE_ENABLED is true. Default is"
+ + " false (catalog state is removed; files are left in place).")
Review Comment:
hmm yes, agreed that is a real risk until unique table locations are the catalog invariant. after the hold we free the name, so a recreate at the same location can see leftover files.
I left the flag default-off for phase 1 so expire matches today's DROP without purge (catalog state gone, files remain) and so opting into hold/recover does not also become delayed file deletion. DROP PURGE on a live table is still immediate destroy, including files.
if you would rather Phase 1 always enqueue entity-cleanup when a soft-deleted table is permanently deleted (drop the flag, or default it true), I can do that. That would make expire match DROP PURGE for files and close the recreate-at-same-location leak while unique locations are still opt-in.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
Posted by "orca-review-bot (via GitHub)" <gi...@apache.org>.
orca-review-bot commented on code in PR #5544:
URL: https://github.com/apache/polaris/pull/5544#discussion_r4383009199
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1408,6 +1408,13 @@ public void dropTableWithoutPurge(TableIdentifier tableIdentifier) {
resolveAndAuthorizeBasicTableLikeOperationOrThrow(
op, PolarisEntitySubType.ICEBERG_TABLE, tableIdentifier);
+ CatalogEntity catalog = getResolvedCatalogEntity();
+ if (realmConfig().getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_ENABLED, catalog)
Review Comment:
If an operator sets `TABLE_SOFT_DELETE_ENABLED=true` realm-wide, every non-purge DROP on a federated/external catalog starts failing with 400. Those catalogs never asked for soft-delete, so this is a pretty surprising regression. Can we just fall through to the normal hard drop for external catalogs instead of rejecting?
<sub>🤖 AI-generated review comment — agents make mistakes, so please verify before acting on it.</sub>
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -3061,6 +3054,113 @@ private void updateTableLike(
purge);
}
+ private boolean tableSoftDeleteEnabled() {
+ return realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_ENABLED, catalogEntity);
+ }
+
+ private long softDeleteHoldPeriodMillis() {
+ String spec =
+ realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_HOLD_PERIOD, catalogEntity);
+ try {
+ long millis = Duration.parse(spec).toMillis();
+ if (millis < 0) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; duration must not be negative", spec);
+ }
+ return millis;
+ } catch (DateTimeException | ArithmeticException e) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; expected an ISO-8601 duration such as P7D",
+ spec);
+ }
+ }
+
+ /**
+ * Permanently delete soft-deleted Iceberg tables in {@code catalogPath} whose hold has expired.
+ * No-op when soft-delete is disabled. Catalog-only: files are purged only if configured.
+ */
+ private void expireEligibleSoftDeletedTables(List<PolarisEntity> catalogPath) {
+ if (!tableSoftDeleteEnabled()) {
+ return;
+ }
+ boolean purgeOnExpire =
+ realmConfig.getConfig(
+ FeatureConfiguration.TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE, catalogEntity);
+ long now = System.currentTimeMillis();
+ List<PolarisBaseEntity> entities =
+ getMetaStoreManager()
+ .listFullEntitiesAll(
+ getCurrentPolarisContext(),
+ PolarisEntity.toCoreList(catalogPath),
+ PolarisEntityType.TABLE_LIKE,
+ PolarisEntitySubType.ICEBERG_TABLE);
+ for (PolarisBaseEntity entity : entities) {
+ if (entity.isDropped()
+ && entity.getToPurgeTimestamp() > 0
+ && now >= entity.getToPurgeTimestamp()) {
+ boolean purgeEntity = purgeOnExpire;
+ Map<String, String> cleanupProperties = Map.of();
+ if (purgeOnExpire) {
+ IcebergTableLikeEntity icebergTableLikeEntity = IcebergTableLikeEntity.of(entity);
+ if (icebergTableLikeEntity == null) {
+ purgeEntity = false;
+ } else {
+ TableIdentifier identifier = icebergTableLikeEntity.getTableIdentifier();
+ String metadataLocation = icebergTableLikeEntity.getMetadataLocation();
+ TableMetadata lastMetadata = null;
+ if (metadataLocation != null) {
+ try {
+ String locationDir =
+ metadataLocation.substring(0, metadataLocation.lastIndexOf('/'));
+ PolarisResolvedPathWrapper resolvedStorage =
+ CatalogUtils.findResolvedStorageEntity(resolvedEntityView, identifier);
+ FileIO tableFileIO =
+ loadFileIOForTableLike(
+ identifier,
+ Set.of(locationDir),
+ resolvedStorage,
+ new HashMap<>(tableDefaultProperties),
+ Set.of(PolarisStorageActions.READ, PolarisStorageActions.LIST));
+ lastMetadata = TableMetadataParser.read(tableFileIO, metadataLocation);
+ } catch (RuntimeException e) {
+ LOGGER.warn(
+ "Unable to read metadata at {} for expired soft-deleted table {}, skipping"
+ + " data purge",
+ metadataLocation,
+ identifier,
+ e);
+ purgeEntity = false;
Review Comment:
If the metadata read fails, we flip `purgeEntity` to false but still hard-delete the entity just below. With purge-on-expire on, the files are orphaned for good and the name is freed even though cleanup never ran. Can we keep the entity in this case so the next expiry pass can retry?
<sub>🤖 AI-generated review comment — agents make mistakes, so please verify before acting on it.</sub>
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -850,6 +827,8 @@ public boolean dropNamespace(Namespace namespace) throws NamespaceNotEmptyExcept
List<PolarisEntity> catalogPath = resolvedEntities.getRawParentPath();
PolarisEntity leafEntity = resolvedEntities.getRawLeafEntity();
+ expireEligibleSoftDeletedTables(resolvedEntities.getRawFullPath());
Review Comment:
During the hold, `dropNamespace` fails with 'Namespace is not empty' while `listTables` returns nothing, which is confusing for users. Can the error say that a soft-deleted table is blocking the drop?
<sub>🤖 AI-generated review comment — agents make mistakes, so please verify before acting on it.</sub>
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -3061,6 +3054,113 @@ private void updateTableLike(
purge);
}
+ private boolean tableSoftDeleteEnabled() {
+ return realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_ENABLED, catalogEntity);
+ }
+
+ private long softDeleteHoldPeriodMillis() {
+ String spec =
+ realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_HOLD_PERIOD, catalogEntity);
+ try {
+ long millis = Duration.parse(spec).toMillis();
+ if (millis < 0) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; duration must not be negative", spec);
+ }
+ return millis;
+ } catch (DateTimeException | ArithmeticException e) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; expected an ISO-8601 duration such as P7D",
+ spec);
+ }
+ }
+
+ /**
+ * Permanently delete soft-deleted Iceberg tables in {@code catalogPath} whose hold has expired.
+ * No-op when soft-delete is disabled. Catalog-only: files are purged only if configured.
+ */
+ private void expireEligibleSoftDeletedTables(List<PolarisEntity> catalogPath) {
+ if (!tableSoftDeleteEnabled()) {
+ return;
+ }
+ boolean purgeOnExpire =
+ realmConfig.getConfig(
+ FeatureConfiguration.TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE, catalogEntity);
+ long now = System.currentTimeMillis();
+ List<PolarisBaseEntity> entities =
+ getMetaStoreManager()
+ .listFullEntitiesAll(
Review Comment:
This loads every full table entity in the namespace on every `listTables` page and every create. Pagination then stops helping on big namespaces, since page 1 of a 10k-table namespace pays the full scan. Can we bound it (cap per call, or only scan when something can be held)?
<sub>🤖 AI-generated review comment — agents make mistakes, so please verify before acting on it.</sub>
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java:
##########
@@ -345,6 +345,30 @@ default BaseResult bootstrapPolarisService(@NonNull PolarisCallContext callCtx)
@Nullable Map<String, String> cleanupProperties,
boolean cleanup);
+ /**
+ * Mark the entity as soft-deleted without removing it from the active name index and without
+ * deleting grants or data files. The identifier stays reserved. Subsequent Iceberg REST list,
+ * load, and HEAD calls treat the entity as not found because {@link
+ * PolarisBaseEntity#isDropped()} is true.
+ *
+ * <p>A later {@link #dropEntityIfExists} call permanently removes catalog state (and may enqueue
+ * file cleanup). If the entity is already soft-deleted, this is a successful no-op.
+ *
+ * @param callCtx call context
+ * @param catalogPath path to that entity
+ * @param entityToDrop entity to soft-delete, must have been resolved by the client
+ * @param dropTimestamp epoch millis when the entity was soft-deleted
+ * @param toPurgeTimestamp epoch millis when the entity becomes eligible for permanent delete
+ */
+ default @NonNull DropEntityResult softDeleteEntityIfExists(
+ @NonNull PolarisCallContext callCtx,
+ @Nullable List<PolarisEntityCore> catalogPath,
+ @NonNull PolarisBaseEntity entityToDrop,
+ long dropTimestamp,
+ long toPurgeTimestamp) {
+ throw new UnsupportedOperationException("softDeleteEntityIfExists");
Review Comment:
Custom metastore managers inherit this throwing default, so enabling the flag on them will 500 on every DROP. Can we say that in the javadoc?
<sub>🤖 AI-generated review comment — agents make mistakes, so please verify before acting on it.</sub>
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -3061,6 +3054,113 @@ private void updateTableLike(
purge);
}
+ private boolean tableSoftDeleteEnabled() {
+ return realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_ENABLED, catalogEntity);
+ }
+
+ private long softDeleteHoldPeriodMillis() {
+ String spec =
+ realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_HOLD_PERIOD, catalogEntity);
+ try {
+ long millis = Duration.parse(spec).toMillis();
+ if (millis < 0) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; duration must not be negative", spec);
+ }
+ return millis;
+ } catch (DateTimeException | ArithmeticException e) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; expected an ISO-8601 duration such as P7D",
+ spec);
+ }
+ }
+
+ /**
+ * Permanently delete soft-deleted Iceberg tables in {@code catalogPath} whose hold has expired.
+ * No-op when soft-delete is disabled. Catalog-only: files are purged only if configured.
+ */
+ private void expireEligibleSoftDeletedTables(List<PolarisEntity> catalogPath) {
+ if (!tableSoftDeleteEnabled()) {
Review Comment:
Once the flag is turned off, expiry never runs again. Any tables that were soft-deleted earlier stay hidden, keep their names reserved, and keep `dropNamespace` failing forever, and `testSoftDeletedTablesStayHiddenWhenSoftDeleteDisabled` locks that in. Should expiry keep running whenever held rows exist, regardless of the flag?
<sub>🤖 AI-generated review comment — agents make mistakes, so please verify before acting on it.</sub>
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
Posted by "iprithv (via GitHub)" <gi...@apache.org>.
iprithv commented on code in PR #5544:
URL: https://github.com/apache/polaris/pull/5544#discussion_r4116155536
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -3004,6 +3023,71 @@ private void updateTableLike(
purge);
}
+ private boolean tableSoftDeleteEnabled() {
+ return realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_ENABLED, catalogEntity);
+ }
+
+ private long softDeleteHoldPeriodMillis() {
+ String spec =
+ realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_HOLD_PERIOD, catalogEntity);
+ try {
+ long millis = Duration.parse(spec).toMillis();
+ if (millis < 0) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; duration must not be negative", spec);
+ }
+ return millis;
+ } catch (DateTimeException | ArithmeticException e) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; expected an ISO-8601 duration such as P7D",
+ spec);
+ }
+ }
+
+ /**
+ * Permanently delete soft-deleted Iceberg tables in {@code catalogPath} whose hold has expired.
+ * No-op when soft-delete is disabled. Catalog-only: files are purged only if configured.
+ */
+ private void expireEligibleSoftDeletedTables(List<PolarisEntity> catalogPath) {
+ if (!tableSoftDeleteEnabled()) {
+ return;
+ }
+ boolean cleanup =
+ realmConfig.getConfig(
+ FeatureConfiguration.TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE, catalogEntity);
+ Map<String, String> cleanupProperties =
+ cleanup ? new HashMap<>(catalogEntity.getInternalPropertiesAsMap()) : Map.of();
Review Comment:
Thanks for catching both issues. @flyingImer
For `listTables` when soft-delete is disabled, we now always use `listFullEntities` and filter out dropped entities. So a table deleted when the flag was on will not show up again after the flag is turned off.
For expire + purge, we no longer pass catalog-only internal props to the cleanup task. Expire now builds the same purge map as `dropTable` (metadata +/ storage hierarchy + `storageLocation`). If metadata read fails, we log a warning and skip the data purge. WDYT?
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
Posted by "iprithv (via GitHub)" <gi...@apache.org>.
iprithv commented on code in PR #5544:
URL: https://github.com/apache/polaris/pull/5544#discussion_r4052674056
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -799,6 +779,44 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(10)
.buildFeatureConfiguration();
+ public static final FeatureConfiguration<Boolean> TABLE_SOFT_DELETE_ENABLED =
+ PolarisConfiguration.<Boolean>builder()
+ .key("TABLE_SOFT_DELETE_ENABLED")
+ .catalogConfig("polaris.config.table-soft-delete.enabled")
+ .description(
+ "If set to true, DROP TABLE without purge marks an INTERNAL Iceberg table as"
+ + " soft-deleted for a hold period instead of permanently removing it. Soft-deleted"
+ + " tables are hidden from Iceberg REST list/load/HEAD, the identifier stays"
+ + " reserved until permanent delete, and data files are not deleted. Default is"
+ + " false (today's hard DROP). DROP TABLE with purge remains an immediate"
+ + " permanent delete.")
+ .defaultValue(false)
+ .buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<String> TABLE_SOFT_DELETE_HOLD_PERIOD =
+ PolarisConfiguration.<String>builder()
+ .key("TABLE_SOFT_DELETE_HOLD_PERIOD")
Review Comment:
phase 1 is lazy on namespace access, not a scheduled sweep (this is also @jbonofre's question on https://lists.apache.org/thread/1djyz0x3k7m71d71bqhw11c3syyshtdr).
when TABLE_SOFT_DELETE_ENABLED is false (the default), expiration is a no-op: a config check only, no extra scan.
when it is enabled, the next list, create, or drop-namespace in that namespace loads Iceberg tables under it (including dropped ones) and permanently deletes those whose toPurgeTimestamp has passed. catalog state is always removed then. file cleanup is a separate flag and uses the existing ENTITY_CLEANUP_SCHEDULER. There is no dedicated expiration task, and no purge-now API, in phase 1.
I spelled that out on TABLE_SOFT_DELETE_HOLD_PERIOD. Cost is confined to catalogs that opt in; a namespace with many held tables pays a full Iceberg-table listing on those operations. scheduled sweep can wait until we know whether lazy-on-access is enough.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
Posted by "flyingImer (via GitHub)" <gi...@apache.org>.
flyingImer commented on code in PR #5544:
URL: https://github.com/apache/polaris/pull/5544#discussion_r4067727531
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -3004,6 +3023,71 @@ private void updateTableLike(
purge);
}
+ private boolean tableSoftDeleteEnabled() {
+ return realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_ENABLED, catalogEntity);
+ }
+
+ private long softDeleteHoldPeriodMillis() {
+ String spec =
+ realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_HOLD_PERIOD, catalogEntity);
+ try {
+ long millis = Duration.parse(spec).toMillis();
+ if (millis < 0) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; duration must not be negative", spec);
+ }
+ return millis;
+ } catch (DateTimeException | ArithmeticException e) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; expected an ISO-8601 duration such as P7D",
+ spec);
+ }
+ }
+
+ /**
+ * Permanently delete soft-deleted Iceberg tables in {@code catalogPath} whose hold has expired.
+ * No-op when soft-delete is disabled. Catalog-only: files are purged only if configured.
+ */
+ private void expireEligibleSoftDeletedTables(List<PolarisEntity> catalogPath) {
+ if (!tableSoftDeleteEnabled()) {
+ return;
+ }
+ boolean cleanup =
+ realmConfig.getConfig(
+ FeatureConfiguration.TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE, catalogEntity);
+ Map<String, String> cleanupProperties =
+ cleanup ? new HashMap<>(catalogEntity.getInternalPropertiesAsMap()) : Map.of();
Review Comment:
IIUC, with purge-on-expiration enabled, the cleanup task is missing the table's storage location. This map only copies catalog internal properties, but TaskFileIOSupplier reads STORAGE_LOCATION from the task and calls Set.of(location), so cleanup fails with a NPE before deleting any files, after the catalog entity is already gone. Please make expiration with purge enabled actually delete the files and cover it through task execution. Reusing the per-table cleanup context assembled by dropTable is one option.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -3198,19 +3282,32 @@ private Page<TableIdentifier> listTableLike(
}
List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath();
- ListEntitiesResult listResult =
- getMetaStoreManager()
- .listEntities(
- getCurrentPolarisContext(),
- PolarisEntity.toCoreList(catalogPath),
- PolarisEntityType.TABLE_LIKE,
- subType,
- pageToken);
+ expireEligibleSoftDeletedTables(catalogPath);
Namespace parentNamespace = PolarisCatalogHelpers.parentNamespace(catalogPath);
- return listResult
- .getPage()
- .map(record -> TableIdentifier.of(parentNamespace, record.getName()));
+ if (!tableSoftDeleteEnabled()) {
Review Comment:
Disabling soft-delete makes previously dropped tables appear in LIST again. Their name-index records are intentionally retained, and this branch returns them without checking isDropped, while LOAD/HEAD still reject them. Please keep existing dropped tables hidden independently of whether new drops are soft-deleted, with an enable/drop/disable/list regression test.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
Posted by "flyingImer (via GitHub)" <gi...@apache.org>.
flyingImer commented on code in PR #5544:
URL: https://github.com/apache/polaris/pull/5544#discussion_r4067718295
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -799,6 +779,44 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(10)
.buildFeatureConfiguration();
+ public static final FeatureConfiguration<Boolean> TABLE_SOFT_DELETE_ENABLED =
+ PolarisConfiguration.<Boolean>builder()
+ .key("TABLE_SOFT_DELETE_ENABLED")
+ .catalogConfig("polaris.config.table-soft-delete.enabled")
+ .description(
+ "If set to true, DROP TABLE without purge marks an INTERNAL Iceberg table as"
+ + " soft-deleted for a hold period instead of permanently removing it. Soft-deleted"
+ + " tables are hidden from Iceberg REST list/load/HEAD, the identifier stays"
+ + " reserved until permanent delete, and data files are not deleted. Default is"
+ + " false (today's hard DROP). DROP TABLE with purge remains an immediate"
+ + " permanent delete.")
+ .defaultValue(false)
+ .buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<String> TABLE_SOFT_DELETE_HOLD_PERIOD =
+ PolarisConfiguration.<String>builder()
+ .key("TABLE_SOFT_DELETE_HOLD_PERIOD")
+ .catalogConfig("polaris.config.table-soft-delete.hold-period")
+ .description(
+ "ISO-8601 duration to retain a soft-deleted Iceberg table before it becomes eligible"
+ + " for permanent delete. Used only when TABLE_SOFT_DELETE_ENABLED is true."
+ + " Default is P7D (seven days).")
+ .defaultValue("P7D")
+ .buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<Boolean>
+ TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE =
+ PolarisConfiguration.<Boolean>builder()
+ .key("TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE")
+ .catalogConfig("polaris.config.table-soft-delete.purge-data-on-permanent-delete")
+ .description(
+ "If set to true, permanent delete of a previously soft-deleted Iceberg table"
+ + " enqueues the existing entity-cleanup task so data and metadata files may"
+ + " be removed. Used only when TABLE_SOFT_DELETE_ENABLED is true. Default is"
+ + " false (catalog state is removed; files are left in place).")
Review Comment:
Enqueueing cleanup on every permanent delete would still leave a name-reuse window. dropEntityIfExists releases the identifier before asynchronous cleanup runs, so a recreate at the same location can succeed while the old files remain. Same-location recreation needs to stay blocked until file cleanup, including the child tasks spawned by the cleanup scheduler, has completed, unless we can guarantee isolation from the retained location. A test with cleanup paused would pin that ordering.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
Posted by "iprithv (via GitHub) via issues" <is...@polaris.apache.org>.
iprithv commented on code in PR #5544:
URL: https://github.com/apache/polaris/pull/5544#discussion_r4433158255
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -3061,6 +3054,113 @@ private void updateTableLike(
purge);
}
+ private boolean tableSoftDeleteEnabled() {
+ return realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_ENABLED, catalogEntity);
+ }
+
+ private long softDeleteHoldPeriodMillis() {
+ String spec =
+ realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_HOLD_PERIOD, catalogEntity);
+ try {
+ long millis = Duration.parse(spec).toMillis();
+ if (millis < 0) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; duration must not be negative", spec);
+ }
+ return millis;
+ } catch (DateTimeException | ArithmeticException e) {
+ throw new BadRequestException(
+ "Invalid table soft-delete hold period '%s'; expected an ISO-8601 duration such as P7D",
+ spec);
+ }
+ }
+
+ /**
+ * Permanently delete soft-deleted Iceberg tables in {@code catalogPath} whose hold has expired.
+ * No-op when soft-delete is disabled. Catalog-only: files are purged only if configured.
+ */
+ private void expireEligibleSoftDeletedTables(List<PolarisEntity> catalogPath) {
+ if (!tableSoftDeleteEnabled()) {
+ return;
+ }
+ boolean purgeOnExpire =
+ realmConfig.getConfig(
+ FeatureConfiguration.TABLE_SOFT_DELETE_PURGE_DATA_ON_PERMANENT_DELETE, catalogEntity);
+ long now = System.currentTimeMillis();
+ List<PolarisBaseEntity> entities =
+ getMetaStoreManager()
+ .listFullEntitiesAll(
+ getCurrentPolarisContext(),
+ PolarisEntity.toCoreList(catalogPath),
+ PolarisEntityType.TABLE_LIKE,
+ PolarisEntitySubType.ICEBERG_TABLE);
+ for (PolarisBaseEntity entity : entities) {
+ if (entity.isDropped()
+ && entity.getToPurgeTimestamp() > 0
+ && now >= entity.getToPurgeTimestamp()) {
+ boolean purgeEntity = purgeOnExpire;
+ Map<String, String> cleanupProperties = Map.of();
+ if (purgeOnExpire) {
+ IcebergTableLikeEntity icebergTableLikeEntity = IcebergTableLikeEntity.of(entity);
+ if (icebergTableLikeEntity == null) {
+ purgeEntity = false;
+ } else {
+ TableIdentifier identifier = icebergTableLikeEntity.getTableIdentifier();
+ String metadataLocation = icebergTableLikeEntity.getMetadataLocation();
+ TableMetadata lastMetadata = null;
+ if (metadataLocation != null) {
+ try {
+ String locationDir =
+ metadataLocation.substring(0, metadataLocation.lastIndexOf('/'));
+ PolarisResolvedPathWrapper resolvedStorage =
+ CatalogUtils.findResolvedStorageEntity(resolvedEntityView, identifier);
+ FileIO tableFileIO =
+ loadFileIOForTableLike(
+ identifier,
+ Set.of(locationDir),
+ resolvedStorage,
+ new HashMap<>(tableDefaultProperties),
+ Set.of(PolarisStorageActions.READ, PolarisStorageActions.LIST));
+ lastMetadata = TableMetadataParser.read(tableFileIO, metadataLocation);
+ } catch (RuntimeException e) {
+ LOGGER.warn(
+ "Unable to read metadata at {} for expired soft-deleted table {}, skipping"
+ + " data purge",
+ metadataLocation,
+ identifier,
+ e);
+ purgeEntity = false;
Review Comment:
Makes sense, that was still deleting the entity. if the metadata read fails we leave it in place now, so the next list or create can retry. the name stays reserved until that works
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] add opt-in Iceberg table soft-delete (#5054) [polaris]
Posted by "orca-review-bot (via GitHub)" <gi...@apache.org>.
orca-review-bot commented on code in PR #5544:
URL: https://github.com/apache/polaris/pull/5544#discussion_r4383009243
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -3061,6 +3054,113 @@ private void updateTableLike(
purge);
}
+ private boolean tableSoftDeleteEnabled() {
+ return realmConfig.getConfig(FeatureConfiguration.TABLE_SOFT_DELETE_ENABLED, catalogEntity);
+ }
+
+ private long softDeleteHoldPeriodMillis() {
Review Comment:
A bad hold-period value is operator config, but it surfaces as a 400 on the end user's DROP. Can we validate it when catalog properties are set instead?
<sub>🤖 AI-generated review comment — agents make mistakes, so please verify before acting on it.</sub>
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org