dimas-b commented on code in PR #4023:
URL: https://github.com/apache/polaris/pull/4023#discussion_r3376611787
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java:
##########
@@ -18,35 +18,57 @@
*/
package org.apache.polaris.service.catalog.io;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
import java.util.Optional;
import org.apache.polaris.core.entity.PolarisEntity;
import org.apache.polaris.core.entity.PolarisEntityConstants;
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
+import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
+import org.apache.polaris.core.storage.StorageConfigOverrideResolver;
public class FileIOUtil {
private FileIOUtil() {}
/**
- * Finds storage configuration information in the hierarchy of the resolved
storage entity.
+ * Finds storage configuration information in the hierarchy of the resolved
storage entity, with
+ * any nearest-ancestor {@code storage_name_override} applied to the
catalog's base config.
*
- * <p>This method starts at the "leaf" level (e.g., table) and walks
"upwards" through namespaces
- * in the hierarchy to the "root." It searches for the first entity
containing storage config
- * properties, identified using a key from {@link
- * PolarisEntityConstants#getStorageConfigInfoPropertyName()}.
+ * <p>This method walks the path leaf-to-root, locates the first entity
carrying {@code
+ * storage_configuration_info} (the catalog), and — if any closer ancestor
carries a {@code
+ * storage_name_override} — returns a synthetic copy of the base entity
whose serialized
+ * configuration has its {@code storageName} replaced. Callers that read the
resulting entity's
+ * internal properties (e.g., to stash on a purge task) therefore see the
effective config.
*
* @param resolvedStorageEntity the resolved entity wrapper containing the
hierarchical path
- * @return an {@link Optional} containing the entity with storage config, or
empty if not found
+ * @return an {@link Optional} containing the (possibly synthetic) entity
with the effective
+ * storage config, or empty if no entity in the chain has a base storage
config
Review Comment:
Callers of this method only need to know the presence of this entity and if
present, its properties... Why bother with synthetic entities? I'd propose to
refactor this code to return an `Optional<some properties class>`... WDYT?
--
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]
