jbonofre commented on code in PR #5095:
URL: https://github.com/apache/polaris/pull/5095#discussion_r3612266800
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java:
##########
@@ -285,6 +285,32 @@ public void deleteEntity(@NonNull PolarisCallContext
callCtx, @NonNull PolarisBa
runActionInTransaction(callCtx, () ->
this.deleteEntityInCurrentTxn(callCtx, entity));
}
+ /** {@inheritDoc} */
+ @Override
+ public void deleteEntityAndCreateEntities(
+ @NonNull PolarisCallContext callCtx,
+ @NonNull PolarisBaseEntity entityToDelete,
+ @NonNull List<PolarisBaseEntity> entitiesToCreate) {
+ runActionInTransaction(
+ callCtx,
+ () -> {
+ this.deleteEntityInCurrentTxn(callCtx, entityToDelete);
+ for (PolarisBaseEntity entityToCreate : entitiesToCreate) {
+ try {
+ this.checkConditionsForWriteEntityInCurrentTxn(callCtx,
entityToCreate, null);
+ } catch (EntityAlreadyExistsException e) {
Review Comment:
This is a behavior change here, as it can now escape `dropEntityIfExists`.
Previously, the code ignored the result of `createEntityIfNotExists`, so a
name collision on the cleanup task silently "succeeded".
Now, `EntityAlreadyExistsException` propagates.
I think it's more "correct", but:
1. It's inconsistent with this class's convention of translating
`EntityAlreadyExistsException` into a result status. We should consider
returning a `DropEntityResult` error status instead of throwing a raw
persistence exception through the manager API.
2. The realistic trigger is a concurrent double-drop of the same entity (two
cleanup tasks named `entityCleanup_<sameId>` with different task ids). The
earlier existence check narrows this window but doesn't fully close it. It's
worth confirming the REST/service layer degrades gracefully (maps to 409, not
500) rather than leaking a stack trace.
--
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]
