(polaris) branch main updated: fix(catalog): encode default table locations with UTF-8 (#4975)

[di...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dimas-b pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git


The following commit(s) were added to refs/heads/main by this push:
     new e9039e1200 fix(catalog): encode default table locations with UTF-8 (#4975)
e9039e1200 is described below

commit e9039e12003a13e783b5130a3d30d30cfe78d93c
Author: Anas Khan <an...@gmail.com>
AuthorDate: Fri Jul 17 04:35:28 2026 +0530

    fix(catalog): encode default table locations with UTF-8 (#4975)
    
    buildPrefixedLocation derived a table's default storage location by
    percent-encoding the namespace levels and table name with
    URLEncoder.encode(..., Charset.defaultCharset()). For non-ASCII
    identifiers the persisted location then depended on the JVM's default
    charset, so the same table could resolve to a different (or lossy)
    location on a non-UTF-8 JVM, breaking portability and round-trip
    expectations.
    
    Encode both segments with StandardCharsets.UTF_8, matching
    PolarisEntityUtils.encodeNamespace and every other URLEncoder call site
    in the service. Add a regression test that creates a table under
    non-ASCII namespace and table identifiers with object-storage prefixing
    enabled and asserts the derived location uses the UTF-8 percent-encoded
    form regardless of the platform default charset.
    
    Signed-off-by: Anas Khan <83...@users.noreply.github.com>
---
 CHANGELOG.md                                       |  1 +
 .../catalog/iceberg/LocalIcebergCatalog.java       |  6 ++--
 .../catalog/iceberg/LocalIcebergCatalogTest.java   | 41 ++++++++++++++++++++--
 3 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 844576b217..bbb33f150e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -61,6 +61,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
 - Deprecated `ALLOW_EXTERNAL_TABLE_LOCATION`. Use `ALLOW_EXTERNAL_METADATA_FILE_LOCATION` for external metadata file locations, including catalog config `polaris.config.allow.external.metadata.file.location`.
 
 ### Fixes
+- Default table storage locations with object-storage prefixing enabled are now percent-encoded with UTF-8 instead of the JVM default charset. Previously the persisted location of a table under a non-ASCII namespace or table name depended on the platform default charset, so the same table could resolve to a different (or lossy) location on a non-UTF-8 JVM.
 - Async task execution (table cleanup, manifest and batch file cleanup) now retries when a handler returns false on transient errors (e.g. IO or delete failures). Previously `false` was swallowed with only a warning log and the task was never retried via the existing retry mechanism.
 - `TableCleanupTaskHandler` now clamps `TABLE_METADATA_CLEANUP_BATCH_SIZE` to at least 1. Previously a non-positive realm override caused an infinite loop (0) or `IllegalArgumentException` (<0) when splitting metadata files for cleanup.
 - `ManifestFileCleanupTaskHandler` now handles Iceberg v2 delete manifests in addition to data manifests. Previously, `DROP TABLE PURGE` on a v2 table that had been updated via merge-on-read DML left position-delete files and their manifests as orphans in object storage; the cleanup task failed silently because `ManifestFiles.read()` rejects delete manifests.
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java
index c4db7a5a7d..5b77cdeb19 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java
@@ -34,7 +34,7 @@ import com.google.common.collect.ImmutableMap;
 import java.io.Closeable;
 import java.io.IOException;
 import java.net.URLEncoder;
-import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -1194,11 +1194,11 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
     locationBuilder.append(LocationUtils.computeHash(tableIdentifier.toString()));
 
     for (String ns : tableIdentifier.namespace().levels()) {
-      locationBuilder.append("/").append(URLEncoder.encode(ns, Charset.defaultCharset()));
+      locationBuilder.append("/").append(URLEncoder.encode(ns, StandardCharsets.UTF_8));
     }
     locationBuilder
         .append("/")
-        .append(URLEncoder.encode(defaultTableLikeName(tableIdentifier), Charset.defaultCharset()))
+        .append(URLEncoder.encode(defaultTableLikeName(tableIdentifier), StandardCharsets.UTF_8))
         .append("/");
     return locationBuilder.toString();
   }
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogTest.java
index a1890e14d1..53fa29b63b 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogTest.java
@@ -19,10 +19,18 @@
 
 package org.apache.polaris.service.catalog.iceberg;
 
+import static org.apache.polaris.core.config.FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP;
+import static org.apache.polaris.core.config.FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION;
+import static org.apache.polaris.core.config.FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED;
+import static org.apache.polaris.core.config.FeatureConfiguration.DEFAULT_UNIQUE_TABLE_LOCATION_ENABLED;
+import static org.apache.polaris.core.config.FeatureConfiguration.OPTIMIZED_SIBLING_CHECK;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.when;
 
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -31,6 +39,7 @@ import org.apache.iceberg.catalog.Namespace;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.polaris.core.PolarisDiagnostics;
 import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.config.RealmConfig;
 import org.apache.polaris.core.context.CallContext;
 import org.apache.polaris.core.entity.CatalogEntity;
 import org.apache.polaris.core.entity.PolarisEntity;
@@ -55,6 +64,7 @@ class LocalIcebergCatalogTest {
 
   @Mock ResolverFactory resolverFactory;
   @Mock CallContext callContext;
+  @Mock RealmConfig realmConfig;
   @Mock PolarisResolutionManifestCatalogView resolvedEntityView;
   @Mock CatalogEntity catalogEntity;
   @Mock PolarisDiagnostics diagnostics;
@@ -73,9 +83,13 @@ class LocalIcebergCatalogTest {
 
   @BeforeEach
   void initMocks() {
+    when(callContext.getRealmConfig()).thenReturn(realmConfig);
+    when(catalogEntity.getName()).thenReturn("catalog");
     when(resolvedEntityView.getResolvedCatalogEntity()).thenReturn(catalogEntity);
-    when(resolverFactory.createResolver(any(), any())).thenReturn(resolver);
-    when(resolver.resolveAll()).thenReturn(new ResolverStatus(ResolverStatus.StatusEnum.SUCCESS));
+    lenient().when(resolverFactory.createResolver(any(), any())).thenReturn(resolver);
+    lenient()
+        .when(resolver.resolveAll())
+        .thenReturn(new ResolverStatus(ResolverStatus.StatusEnum.SUCCESS));
 
     rns1 = new ResolvedPolarisEntity(ns1, List.of(), List.of());
     rns2 = new ResolvedPolarisEntity(ns2, List.of(), List.of());
@@ -96,6 +110,29 @@ class LocalIcebergCatalogTest {
             null);
   }
 
+  @Test
+  void testHashedTableLocationsEncodeNonAsciiIdentifiersAsUtf8() {
+    when(realmConfig.getConfig(DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED, catalogEntity))
+        .thenReturn(true);
+    when(realmConfig.getConfig(ALLOW_UNSTRUCTURED_TABLE_LOCATION, catalogEntity)).thenReturn(true);
+    when(realmConfig.getConfig(ALLOW_TABLE_LOCATION_OVERLAP, catalogEntity)).thenReturn(true);
+    when(realmConfig.getConfig(OPTIMIZED_SIBLING_CHECK, catalogEntity)).thenReturn(false);
+    when(realmConfig.getConfig(DEFAULT_UNIQUE_TABLE_LOCATION_ENABLED, catalogEntity))
+        .thenReturn(false);
+    catalog.initialize("catalog", Map.of("warehouse", "s3://bucket/base"));
+
+    String namespace = "n\u00e9\u6f22";
+    String table = "t\u00e9\u6f22";
+    String location = catalog.defaultWarehouseLocation(TableIdentifier.of(namespace, table));
+
+    String expectedSuffix =
+        String.format(
+            "/%s/%s/",
+            URLEncoder.encode(namespace, StandardCharsets.UTF_8),
+            URLEncoder.encode(table, StandardCharsets.UTF_8));
+    assertThat(location).endsWith(expectedSuffix);
+  }
+
   @Test
   void testFailedLocationSiblingResolutionException() {
     when(resolver.resolveAll())