dimas-b commented on code in PR #1139: URL: https://github.com/apache/polaris/pull/1139#discussion_r1988308045
########## 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: nit: this `null` appears to be returned on parsing exceptions. This looks odd to me... Whether `getDiagServices().fail()` throws is very non-intuitive. ########## 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: I believe this approach implies that the transaction isolation level is "serializable", which is fine from my POV, I just want to confirm my understanding. -- 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