dimas-b commented on code in PR #5094:
URL: https://github.com/apache/polaris/pull/5094#discussion_r3752847002
##########
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:
This is not quite what I meant 😅 We catch `RuntimeException` and then have
`if`s on the sub-type... This is not nice. Why not have a separate
`catch(EntityAlreadyExistsException | RetryOnConcurrencyException)` above line
391?
--
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]
