flyrain commented on code in PR #5134:
URL: https://github.com/apache/polaris/pull/5134#discussion_r3648925565
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java:
##########
@@ -143,15 +159,13 @@ public void writeEntities(
PolarisBaseEntity entity = entities.get(i);
PolarisBaseEntity originalEntity =
originalEntities != null ? originalEntities.get(i) : null;
- // first, check if the entity has already been created, in which
case we will simply
- // return it.
- PolarisBaseEntity entityFound =
- lookupEntity(
- callCtx, entity.getCatalogId(), entity.getId(),
entity.getTypeCode());
- if (entityFound != null && originalEntity == null) {
- // probably the client retried, simply return it
- // TODO: Check correctness of returning entityFound vs entity
here. It may have
- // already been updated after the creation.
+ // On the create path (no known original entity) a client retry
may be replaying a
+ // create that already succeeded; skip it so the write stays
idempotent. Only
+ // existence matters here, so issue a cheap EXISTS check instead
of fetching the full
+ // row (including the large JSON property blobs). On the update
path the original
+ // entity is already known, so this check is unnecessary and is
skipped entirely.
Review Comment:
Nit: I think these comments are unnecessary. Sorry, didn't catch it in the
first round.
--
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]
