fix(jdbc): handle trailing-slash mismatch in optimized sibling overlap check

CHANGELOG.md

@@ -64,6 +64,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
 - `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.
 - OPA authorizer now includes the realm identifier in the authorization context sent to OPA (`input.context.realm`). This ensures OPA policies can enforce tenant isolation across realms, preventing potential collisions if identical principal or resource names exist in different realms.
+- Fixed a false-negative in the JDBC optimized location-overlap check (`OPTIMIZED_SIBLING_CHECK`). Ancestor locations stored in `location_without_scheme` without a trailing slash were not matched by the generated ancestor equality terms, allowing nested table/namespace locations to be created under existing prefixes. The query now emits both slash-terminated and non-slash-terminated prefix terms and uses a slash-terminated `LIKE` pattern for descendant matching.
 
 ## [1.6.0]
 

persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java

@@ -23,6 +23,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -390,6 +391,10 @@ static PreparedQuery generateEntityTableExistQuery() {
    * a path on one storage type may give a false positive for overlapping with another storage type.
    * This should be combined with a check using `StorageLocation`.
    *
+   * <p>Equality terms are generated for each prefix of the location in both slash-terminated and
+   * non-slash-terminated forms so that ancestors stored with or without a trailing slash are both
+   * matched.
+   *
    * @param realmId A realm to search within
    * @param schemaVersion The schema version of entities table to query
    * @param catalogId A catalog entity to search within
@@ -405,18 +410,35 @@ public static PreparedQuery generateOverlapQuery(
     List<String> conditions = new ArrayList<>();
     List<Object> parameters = new ArrayList<>();
 
-    String[] components = locationWithoutScheme.split("/");
-    StringBuilder pathBuilder = new StringBuilder();
+    // Normalize by stripping any trailing slash so the scan below only emits meaningful prefixes.
+    // The slash-terminated form is re-added as the last prefix term below.
+    String normalizedLocation = locationWithoutScheme;
+    if (normalizedLocation.endsWith("/")) {
+      normalizedLocation = normalizedLocation.substring(0, normalizedLocation.length() - 1);
+    }
+
+    Set<String> prefixTerms = new LinkedHashSet<>();
+    for (int i = 0; i < normalizedLocation.length(); i++) {
+      if (normalizedLocation.charAt(i) == '/') {
+        if (i > 0) {
+          prefixTerms.add(normalizedLocation.substring(0, i));
+        }
+        prefixTerms.add(normalizedLocation.substring(0, i + 1));
+      }
+    }
+    prefixTerms.add(normalizedLocation);
+    prefixTerms.add(normalizedLocation + "/");
 
-    for (String component : components) {
-      pathBuilder.append(component).append("/");
+    for (String prefix : prefixTerms) {
       conditions.add("location_without_scheme = ?");
-      parameters.add(pathBuilder.toString());
+      parameters.add(prefix);
     }
 
-    // Add LIKE condition to match children
+    // Add LIKE condition to match children. Slash-terminate the location so the pattern only
+    // matches true descendants (e.g. //bucket/ns/tA/% matches //bucket/ns/tA/child but not
+    // //bucket/ns/tA_backup).
     conditions.add("location_without_scheme LIKE ?");
-    parameters.add(locationWithoutScheme + "%");
+    parameters.add(StorageLocation.ensureTrailingSlash(locationWithoutScheme) + "%");
 
     String locationClause = String.join(" OR ", conditions);
     String clause = " WHERE realm_id = ? AND catalog_id = ? AND (" + locationClause + ")";

persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/QueryGeneratorTest.java

@@ -390,6 +390,7 @@ void testGenerateOverlapQuery() {
             + " properties, internal_properties, grant_records_version, location_without_scheme FROM"
             + " POLARIS_SCHEMA.ENTITIES WHERE realm_id = ? AND catalog_id = ? AND (location_without_scheme = ?"
             + " OR location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR"
+            + " location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR"
             + " location_without_scheme = ? OR location_without_scheme LIKE ?)",
         QueryGenerator.generateOverlapQuery("realmId", 2, -123, "s3://bucket/tmp/location/").sql());
     Assertions.assertThatCollection(
@@ -400,34 +401,75 @@ void testGenerateOverlapQuery() {
             -123L,
             "/",
             "//",
+            "//bucket",
             "//bucket/",
+            "//bucket/tmp",
             "//bucket/tmp/",
+            "//bucket/tmp/location",
+            "//bucket/tmp/location/",
+            "//bucket/tmp/location/%");
+
+    // A location without a trailing slash produces the same prefix terms so that ancestors stored
+    // in either form are matched.
+    Assertions.assertThatCollection(
+            QueryGenerator.generateOverlapQuery("realmId", 2, -123, "s3://bucket/tmp/location")
+                .parameters())
+        .containsExactly(
+            "realmId",
+            -123L,
+            "/",
+            "//",
+            "//bucket",
+            "//bucket/",
+            "//bucket/tmp",
+            "//bucket/tmp/",
+            "//bucket/tmp/location",
             "//bucket/tmp/location/",
             "//bucket/tmp/location/%");
 
     assertEquals(
         "SELECT id, catalog_id, parent_id, type_code, name, entity_version, sub_type_code,"
             + " create_timestamp, drop_timestamp, purge_timestamp, to_purge_timestamp, last_update_timestamp,"
             + " properties, internal_properties, grant_records_version, location_without_scheme FROM"
-            + " POLARIS_SCHEMA.ENTITIES WHERE realm_id = ? AND catalog_id = ? AND (location_without_scheme = ? OR location_without_scheme = ?"
-            + " OR location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme LIKE ?)",
+            + " POLARIS_SCHEMA.ENTITIES WHERE realm_id = ? AND catalog_id = ? AND (location_without_scheme = ?"
+            + " OR location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR"
+            + " location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR"
+            + " location_without_scheme LIKE ?)",
         QueryGenerator.generateOverlapQuery("realmId", 2, -123, "/tmp/location/").sql());
     Assertions.assertThatCollection(
             QueryGenerator.generateOverlapQuery("realmId", 2, -123, "/tmp/location/").parameters())
         .containsExactly(
-            "realmId", -123L, "/", "//", "///", "///tmp/", "///tmp/location/", "///tmp/location/%");
+            "realmId",
+            -123L,
+            "/",
+            "//",
+            "///",
+            "///tmp",
+            "///tmp/",
+            "///tmp/location",
+            "///tmp/location/",
+            "///tmp/location/%");
 
     assertEquals(
         "SELECT id, catalog_id, parent_id, type_code, name, entity_version, sub_type_code,"
             + " create_timestamp, drop_timestamp, purge_timestamp, to_purge_timestamp, last_update_timestamp,"
             + " properties, internal_properties, grant_records_version, location_without_scheme"
             + " FROM POLARIS_SCHEMA.ENTITIES WHERE realm_id = ? AND catalog_id = ? AND (location_without_scheme = ?"
-            + " OR location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme LIKE ?)",
+            + " OR location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR"
+            + " location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme LIKE ?)",
         QueryGenerator.generateOverlapQuery("realmId", 2, -123, "s3://バケツ/\"loc.ation\"/").sql());
     Assertions.assertThatCollection(
             QueryGenerator.generateOverlapQuery("realmId", 2, -123, "s3://バケツ/\"loc.ation\"/")
                 .parameters())
         .containsExactly(
-            "realmId", -123L, "/", "//", "//バケツ/", "//バケツ/\"loc.ation\"/", "//バケツ/\"loc.ation\"/%");
+            "realmId",
+            -123L,
+            "/",
+            "//",
+            "//バケツ",
+            "//バケツ/",
+            "//バケツ/\"loc.ation\"",
+            "//バケツ/\"loc.ation\"/",
+            "//バケツ/\"loc.ation\"/%");
   }
 }

runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogOverlapTest.java

@@ -19,6 +19,7 @@
 package org.apache.polaris.service.catalog.iceberg;
 
 import static org.apache.polaris.service.admin.PolarisAuthzTestBase.SCHEMA;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.when;
 
@@ -294,4 +295,43 @@ public void testParentChildLocationOverlapWithOptimizedSiblingCheck() {
         .hasMessageContaining("Unable to create entity at location")
         .hasMessageContaining("conflicts with existing table or namespace");
   }
+
+  @Test
+  public void testParentPrefixOverlapWithTrailingSlashMismatch() {
+    // Profiles disable ADD_TRAILING_SLASH_TO_LOCATION so the parent is stored without a trailing
+    // slash. That is the OPTIMIZED_SIBLING_CHECK false-negative for JDBC: ancestor equality terms
+    // used to be slash-terminated only, so location_without_scheme without '/' was missed.
+    Namespace ns = Namespace.of("ns-for-trailing-slash-overlap");
+    catalog().createNamespace(ns);
+
+    TableIdentifier parentTable = TableIdentifier.of(ns, "trailing-parent");
+    String parentLoc = STORAGE_LOCATION + "/trailing-overlap/parent";
+    assertThat(parentLoc).doesNotEndWith("/");
+    catalog().buildTable(parentTable, SCHEMA).withLocation(parentLoc).create();
+
+    // Guardrail: if trailing-slash normalization were still on, this test would not exercise the
+    // slash-less location_without_scheme path that QueryGenerator must handle.
+    assertThat(catalog().loadTable(parentTable).location())
+        .as("parent must remain slash-less so overlap uses non-slash-terminated stored location")
+        .isEqualTo(parentLoc)
+        .doesNotEndWith("/");
+
+    // Creating a child table under the slash-less parent location must be rejected.
+    TableIdentifier childTable = TableIdentifier.of(ns, "trailing-child");
+    String childLoc = STORAGE_LOCATION + "/trailing-overlap/parent/child";
+    assertThatThrownBy(
+            () -> catalog().buildTable(childTable, SCHEMA).withLocation(childLoc).create())
+        .isInstanceOf(ForbiddenException.class)
+        .hasMessageContaining("Unable to create entity at location")
+        .hasMessageContaining("conflicts with existing table or namespace");
+
+    // The same must hold when the slash-terminated form of the same prefix is used.
+    TableIdentifier siblingTable = TableIdentifier.of(ns, "trailing-sibling");
+    String siblingLoc = STORAGE_LOCATION + "/trailing-overlap/parent/";
+    assertThatThrownBy(
+            () -> catalog().buildTable(siblingTable, SCHEMA).withLocation(siblingLoc).create())
+        .isInstanceOf(ForbiddenException.class)
+        .hasMessageContaining("Unable to create entity at location")
+        .hasMessageContaining("conflicts with existing table or namespace");
+  }
 }

runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogNoSqlOverlapTest.java

@@ -46,6 +46,8 @@ public Map<String, String> getConfigOverrides() {
       overrides.put("polaris.features.\"ALLOW_TABLE_LOCATION_OVERLAP\"", "false");
       overrides.put("polaris.features.\"OPTIMIZED_SIBLING_CHECK\"", "true");
       overrides.put("polaris.features.\"ALLOW_OPTIMIZED_SIBLING_CHECK\"", "true");
+      // Keep locations as written so slash-less base locations are stored as-is.
+      overrides.put("polaris.features.\"ADD_TRAILING_SLASH_TO_LOCATION\"", "false");
       return overrides;
     }
   }

runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogOverlapTest.java

@@ -44,6 +44,8 @@ public Map<String, String> getConfigOverrides() {
       overrides.put("polaris.features.\"ALLOW_TABLE_LOCATION_OVERLAP\"", "false");
       overrides.put("polaris.features.\"OPTIMIZED_SIBLING_CHECK\"", "true");
       overrides.put("polaris.features.\"ALLOW_OPTIMIZED_SIBLING_CHECK\"", "true");
+      // Keep locations as written so slash-less base locations are stored as-is.
+      overrides.put("polaris.features.\"ADD_TRAILING_SLASH_TO_LOCATION\"", "false");
       return overrides;
     }
   }

runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogRelationalOverlapTest.java

@@ -45,6 +45,9 @@ public Map<String, String> getConfigOverrides() {
       overrides.put("polaris.features.\"ALLOW_TABLE_LOCATION_OVERLAP\"", "false");
       overrides.put("polaris.features.\"OPTIMIZED_SIBLING_CHECK\"", "true");
       overrides.put("polaris.features.\"ALLOW_OPTIMIZED_SIBLING_CHECK\"", "true");
+      // Keep locations as written so JDBC optimized sibling checks exercise slash-less
+      // location_without_scheme values (the false-negative fixed in QueryGenerator).
+      overrides.put("polaris.features.\"ADD_TRAILING_SLASH_TO_LOCATION\"", "false");
       overrides.put("polaris.persistence.type", "relational-jdbc");
       overrides.put("polaris.persistence.auto-bootstrap-types", "relational-jdbc");
       overrides.put("quarkus.datasource.db-kind", "h2");