singhpk234 commented on code in PR #3334:
URL: https://github.com/apache/polaris/pull/3334#discussion_r2653711937
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java:
##########
@@ -189,21 +203,10 @@ private void persistEntity(
realmId));
} catch (SQLException e) {
if (datasourceOperations.isConstraintViolation(e)) {
- PolarisBaseEntity existingEntity =
- lookupEntityByName(
- callCtx,
- entity.getCatalogId(),
- entity.getParentId(),
- entity.getTypeCode(),
- entity.getName());
- // This happens in two scenarios:
- // 1. PRIMARY KEY violated
- // 2. UNIQUE CONSTRAINT on (realm_id, catalog_id, parent_id,
type_code, name) violated
- // With SERIALIZABLE isolation, the conflicting entity may _not_ be
visible and
- // existingEntity can be null, which would cause an NPE in
- // EntityAlreadyExistsException.message().
- throw new EntityAlreadyExistsException(
- existingEntity != null ? existingEntity : entity, e);
Review Comment:
PG aborts the transaction when it encounters the error :
```
If a statement within a transaction raises an exception, the entire
transaction is
aborted and all statements are rolled back. The transaction remains in an
aborted state,
and no further SQL commands will be executed until a ROLLBACK command is
issued.
```
hence a lookup after write failure is not possible, the tests failed because
of this.
I changed to read before write attempt lets say we are working with
SERIALIZABLE isolation would work now. I see and think more it was intentional
:'( as the comment says, to do it this way worst case was we not able to get
existing entity which would have been fine.
Now lookup before before write is gonna cause some perf issue (but only for
certain cases) and wrapping writeEntity would elevate the scenario more, will
need to run some benchmark before we check it 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
