iprithv commented on code in PR #5094:
URL: https://github.com/apache/polaris/pull/5094#discussion_r3753572522
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -388,12 +389,20 @@ <T> T withRetries(Operation<T> operation) throws
SQLException {
try {
return operation.execute();
} catch (SQLException | RuntimeException e) {
+ // Pass-through domain exceptions unchanged. Do not unwrap their
SQLException cause into the
+ // retry path (that would rewrap them as a generic SQLException and
lose the typed signal
+ // upper layers need, e.g. ENTITY_ALREADY_EXISTS mapping).
+ if (e instanceof EntityAlreadyExistsException) {
Review Comment:
Makes sense 😅 - switched to a dedicated catch
(`EntityAlreadyExistsException` | `RetryOnConcurrencyException e`) { `throw e;
`} clause above the broad catch and dropped the instanceof checks entirely.
Also added `AtomicOperationMetaStoreManagerCreateConflictTest` covering the
`RetryOnConcurrencyException `→ `ENTITY_ALREADY_EXISTS` mapping for both
`createEntityIfNotExists` and `createEntitiesIfNotExist`. PTAL, thanks!
--
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]
