You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by di...@apache.org on 2026/08/10 20:14:29 UTC
(polaris) branch main updated: fix(jdbc): handle trailing-slash mismatch in optimized sibling overlap check (#5003)
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 735f651f42 fix(jdbc): handle trailing-slash mismatch in optimized sibling overlap check (#5003)
735f651f42 is described below
commit 735f651f42211e763bbbb94be8557ff453d34407
Author: vignesh_A <16...@users.noreply.github.com>
AuthorDate: Tue Aug 11 01:44:23 2026 +0530
fix(jdbc): handle trailing-slash mismatch in optimized sibling overlap check (#5003)
* fix(jdbc): handle trailing-slash mismatch in optimized sibling overlap check
* test(jdbc): make trailing-slash overlap test depend on QueryGenerator fix
Disable ADD_TRAILING_SLASH_TO_LOCATION in overlap profiles so parent
locations are stored without a trailing slash, and assert the stored
location remains slash-less. Without that, default slash normalization
masked the OPTIMIZED_SIBLING_CHECK false negative this PR fixes.
---
CHANGELOG.md | 1 +
.../relational/jdbc/QueryGenerator.java | 36 ++++++++++++---
.../relational/jdbc/QueryGeneratorTest.java | 52 +++++++++++++++++++---
.../AbstractLocalIcebergCatalogOverlapTest.java | 40 +++++++++++++++++
.../LocalIcebergCatalogNoSqlOverlapTest.java | 2 +
.../iceberg/LocalIcebergCatalogOverlapTest.java | 2 +
.../LocalIcebergCatalogRelationalOverlapTest.java | 3 ++
7 files changed, 124 insertions(+), 12 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a8d9f92b5c..94abfaa553 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -114,6 +114,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
- Management API delete operations for principals, principal roles, catalog roles, and catalogs now return error messages that match the actual failure reason (for example, concurrent modification no longer reports a misleading protected-entity message).
- Python CLI `setup apply` now defaults to an `INTERNAL` catalog type when the `type` field is left blank or null in the setup config, instead of crashing with `AttributeError`
- Ptyhon CLI `setup` now preserves `endpoint_internal` and `sts_endpoint` during apply and export for S3 configuration
+- 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]
diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java
index 2976fe6e20..aa93ca3531 100644
--- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java
+++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java
@@ -23,6 +23,7 @@ import java.util.ArrayList;
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;
@@ -367,6 +368,10 @@ public class QueryGenerator {
* 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
@@ -382,18 +387,35 @@ public class QueryGenerator {
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 + ")";
diff --git a/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/QueryGeneratorTest.java b/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/QueryGeneratorTest.java
index 913a88db58..ddaf77b7b9 100644
--- a/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/QueryGeneratorTest.java
+++ b/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/QueryGeneratorTest.java
@@ -323,6 +323,7 @@ public class QueryGeneratorTest {
+ " 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(
@@ -333,8 +334,29 @@ public class QueryGeneratorTest {
-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/%");
@@ -342,26 +364,46 @@ public class QueryGeneratorTest {
"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\"/%");
}
@Test
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogOverlapTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogOverlapTest.java
index 9a3f40ab15..ee90c8b3b2 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogOverlapTest.java
+++ b/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;
@@ -295,4 +296,43 @@ public abstract class AbstractLocalIcebergCatalogOverlapTest {
.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");
+ }
}
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogNoSqlOverlapTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogNoSqlOverlapTest.java
index 8bb3aa5bb5..3c8490a6e0 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogNoSqlOverlapTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogNoSqlOverlapTest.java
@@ -46,6 +46,8 @@ public class LocalIcebergCatalogNoSqlOverlapTest extends AbstractLocalIcebergCat
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;
}
}
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogOverlapTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogOverlapTest.java
index a66101a450..7646600413 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogOverlapTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogOverlapTest.java
@@ -44,6 +44,8 @@ public class LocalIcebergCatalogOverlapTest extends AbstractLocalIcebergCatalogO
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;
}
}
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogRelationalOverlapTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogRelationalOverlapTest.java
index 946174d9c4..139974fbdd 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogRelationalOverlapTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogRelationalOverlapTest.java
@@ -45,6 +45,9 @@ public class LocalIcebergCatalogRelationalOverlapTest
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");