dennishuo commented on code in PR #243:
URL: https://github.com/apache/polaris/pull/243#discussion_r1776300626
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java:
##########
@@ -257,6 +260,12 @@ public <T> T runInTransaction(
} finally {
localSession.remove();
}
+ } catch (PersistenceException e) {
+ if (e.toString().toLowerCase(Locale.ROOT).contains("duplicate key")) {
+ throw new AlreadyExistsException(e, "Duplicate key error when
persisting entity");
Review Comment:
I'd prefer adding the mapping to `IcebergExceptionMapper` and explicitly
throwing `jakarta.persistence.EntityExistsException` here. Even though we have
some loose leaking between jakarta exceptions vs Iceberg exceptions in some
places it'll be nice to start segmenting them better depending on whether the
code is agnostic to Iceberg.
Classes that implement Iceberg interfaces directly should stick to Iceberg
exceptions, and ideally the metastore layer can be kept as Iceberg-agnostic as
possible.
--
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]
