dennishuo commented on code in PR #1139: URL: https://github.com/apache/polaris/pull/1139#discussion_r1988346500
########## polaris-core/src/main/java/org/apache/polaris/core/persistence/BaseMetaStoreManager.java: ########## @@ -45,4 +56,179 @@ public static PolarisStorageConfigurationInfo extractStorageConfiguration( return PolarisStorageConfigurationInfo.deserialize( callCtx.getDiagServices(), storageConfigInfoStr); } + + /** + * Given the internal property as a map of key/value pairs, serialize it to a String + * + * @param properties a map of key/value pairs + * @return a String, the JSON representation of the map + */ + public String serializeProperties(PolarisCallContext callCtx, Map<String, String> properties) { + + String jsonString = null; + try { + // Deserialize the JSON string to a Map<String, String> + jsonString = MAPPER.writeValueAsString(properties); + } catch (JsonProcessingException ex) { + callCtx.getDiagServices().fail("got_json_processing_exception", "ex={}", ex); + } + + return jsonString; + } + + /** + * Given the serialized properties, deserialize those to a {@code Map<String, String>} + * + * @param properties a JSON string representing the set of properties + * @return a Map of string + */ + public Map<String, String> deserializeProperties(PolarisCallContext callCtx, String properties) { + + Map<String, String> retProperties = null; Review Comment: Yeah that's a good point; for now these are moved from `PolarisMetaStoreManagerImpl.java` into this shared code, so I'm trying to just preserve the old (maybe buggy) behavior in this PR. I think we have some redundant serialize/deserialize helpers in various places so maybe we can consolidate in one place in a followup. ########## polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java: ########## @@ -194,6 +117,547 @@ protected abstract void deleteFromEntitiesActive( * @param callCtx call context * @param entity entity record to delete */ - protected abstract void deleteFromEntitiesChangeTracking( + protected abstract void deleteFromEntitiesChangeTrackingInCurrentTxn( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore entity); + + // + // Implementations of the one-shot atomic BasePersistence methods which explicitly run + // the in-transaction variants of methods in a new transaction. + // + + /** {@inheritDoc} */ + @Override + public long generateNewId(@Nonnull PolarisCallContext callCtx) { + return runInTransaction(callCtx, () -> this.generateNewIdInCurrentTxn(callCtx)); + } + + /** Helper to perform the compare-and-swap semantics of a single writeEntity call. */ + private void checkConditionsForWriteEntityInCurrentTxn( + @Nonnull PolarisCallContext callCtx, + @Nonnull PolarisBaseEntity entity, + @Nullable PolarisBaseEntity originalEntity) { + PolarisBaseEntity refreshedEntity = + this.lookupEntityInCurrentTxn( + callCtx, entity.getCatalogId(), entity.getId(), entity.getTypeCode()); + + if (originalEntity == null) { + if (refreshedEntity != null) { + // If this is a "create", and we manage to look up an existing entity with already + // the same id, where ids are uniquely reserved when generated, it means it's a + // low-level retry possibly in the face of a transient connectivity failure to + // the backend database. + throw new EntityAlreadyExistsException(refreshedEntity); + } else { + // Successfully verified the entity doesn't already exist by-id, but for a "create" + // we must also check for name-collection now. + refreshedEntity = + this.lookupEntityByNameInCurrentTxn( + callCtx, + entity.getCatalogId(), + entity.getParentId(), + entity.getType().getCode(), + entity.getName()); + if (refreshedEntity != null) { + // Name-collision conflict. + throw new EntityAlreadyExistsException(refreshedEntity); + } + } + } else { + // This is an "update". + if (refreshedEntity == null + || refreshedEntity.getEntityVersion() != originalEntity.getEntityVersion() + || refreshedEntity.getGrantRecordsVersion() != originalEntity.getGrantRecordsVersion()) { Review Comment: Correct -- 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: issues-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org