diff --git a/CHANGELOG.md b/CHANGELOG.md index 37bfbd85df..a3fb6d8608 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti - Fixed JDBC persistence under `SERIALIZABLE` isolation (e.g. CockroachDB default) so that a concurrent entity create that loses a unique-name race no longer returns the phantom new entity as a successful create. The conflicting row is now reported as `ENTITY_ALREADY_EXISTS` instead of fabricating the entity that was not persisted. - Python 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. +- JDBC optimized location-overlap queries no longer include slash-only prefix terms such as `/` and `//`. Those were artifacts of stripping the URI scheme (e.g. `s3://bucket/path` → `//bucket/path`) and are not meaningful storage locations. ### Commits 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 c57105cc34..a7ebcf82a0 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 @@ -395,7 +395,9 @@ static PreparedQuery generateEntityTableExistQuery() { * *

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. + * matched. Prefix terms that consist only of {@code /} characters (other than {@code ///}, the + * root of {@code file:} URIs) are skipped, as they are artifacts of scheme stripping, not + * meaningful storage locations. * * @param realmId A realm to search within * @param schemaVersion The schema version of entities table to query @@ -432,6 +434,11 @@ public static PreparedQuery generateOverlapQuery( prefixTerms.add(normalizedLocation + "/"); for (String prefix : prefixTerms) { + // Skip "/" and "//" produced from empty path segments around the scheme separator; those + // are not meaningful storage locations. "///" (the root of file: URIs) is kept. + if (prefix.length() < 3 && isSlashOnly(prefix)) { + continue; + } conditions.add("location_without_scheme = ?"); parameters.add(prefix); } @@ -441,7 +448,6 @@ public static PreparedQuery generateOverlapQuery( // //bucket/ns/tA_backup). conditions.add("location_without_scheme LIKE ?"); parameters.add(StorageLocation.ensureTrailingSlash(locationWithoutScheme) + "%"); - String locationClause = String.join(" OR ", conditions); String clause = " WHERE realm_id = ? AND catalog_id = ? AND (" + locationClause + ")"; @@ -461,6 +467,20 @@ public static PreparedQuery generateOverlapQuery( return new PreparedQuery(query.sql(), where.parameters()); } + /** True when {@code value} is non-empty and contains only {@code /} characters. */ + @VisibleForTesting + static boolean isSlashOnly(String value) { + if (value == null || value.isEmpty()) { + return false; + } + for (int i = 0; i < value.length(); i++) { + if (value.charAt(i) != '/') { + return false; + } + } + return true; + } + static String getFullyQualifiedTableName(String tableName) { // TODO: make schema name configurable. return "POLARIS_SCHEMA." + tableName; 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 ddaf77b7b9..0615e7ee3f 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 @@ -317,14 +317,15 @@ void testGenerateWhereClauseExtended_allPredicatesAndStableParameterOrder() { @Test void testGenerateOverlapQuery() { + // s3://bucket/tmp/location/ → withoutScheme //bucket/tmp/location/ + // Slash-only prefixes "/" and "//" from the scheme separator are not emitted. 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 = ? OR location_without_scheme = ? OR" - + " location_without_scheme = ? OR location_without_scheme LIKE ?)", + + " location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme LIKE ?)", QueryGenerator.generateOverlapQuery("realmId", 2, -123, "s3://bucket/tmp/location/").sql()); Assertions.assertThatCollection( QueryGenerator.generateOverlapQuery("realmId", 2, -123, "s3://bucket/tmp/location/") @@ -332,8 +333,6 @@ void testGenerateOverlapQuery() { .containsExactly( "realmId", -123L, - "/", - "//", "//bucket", "//bucket/", "//bucket/tmp", @@ -350,8 +349,6 @@ void testGenerateOverlapQuery() { .containsExactly( "realmId", -123L, - "/", - "//", "//bucket", "//bucket/", "//bucket/tmp", @@ -360,22 +357,21 @@ void testGenerateOverlapQuery() { "//bucket/tmp/location/", "//bucket/tmp/location/%"); + // Absolute path becomes file:///tmp/location/ → withoutScheme ///tmp/location/ + // Slash-only prefixes "/" and "//" are not emitted; "///" (the file: root) is kept. 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 = ? OR location_without_scheme = ? OR" - + " location_without_scheme LIKE ?)", + + " 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/", @@ -388,8 +384,7 @@ void testGenerateOverlapQuery() { + " 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 = ? OR location_without_scheme LIKE ?)", + + " 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\"/") @@ -397,8 +392,6 @@ void testGenerateOverlapQuery() { .containsExactly( "realmId", -123L, - "/", - "//", "//バケツ", "//バケツ/", "//バケツ/\"loc.ation\"", @@ -406,6 +399,26 @@ void testGenerateOverlapQuery() { "//バケツ/\"loc.ation\"/%"); } + @Test + void generateOverlapQueryDoesNotEmitSlashOnlyPrefixes() { + // s3-style locations: "/" and "//" artifacts of scheme stripping are not emitted. + Assertions.assertThat( + QueryGenerator.generateOverlapQuery("realmId", 2, -123, "s3://bucket/ns/t/") + .parameters()) + .doesNotContain("/", "//"); + // Same without a trailing slash (prefix walk normalizes; slash-only skip is unchanged). + Assertions.assertThat( + QueryGenerator.generateOverlapQuery("realmId", 2, -123, "s3://bucket/ns/t") + .parameters()) + .doesNotContain("/", "//"); + // file: locations: the "///" root is kept, but "/" and "//" are not emitted. + Assertions.assertThat( + QueryGenerator.generateOverlapQuery("realmId", 2, -123, "file:///tmp/data/") + .parameters()) + .contains("///") + .doesNotContain("/", "//"); + } + @Test void testGenerateExistsQuery() { Map params = new HashMap<>();