From 0b8218b01f2ce84d4766d804c140fac7bd145a55 Mon Sep 17 00:00:00 2001 From: Arnaud Lacurie Date: Mon, 17 Nov 2025 23:44:23 +0000 Subject: [PATCH] Quote identifier components in toString() for clearer error messages Identifier.toString() now wraps each component with double quotes, making error messages more consistent with SQL identifier syntax and improving readability. --- .../relational/recordlayer/query/Identifier.java | 7 ++++++- .../recordlayer/query/SemanticAnalyzer.java | 2 +- .../foundationdb/relational/api/ddl/IndexTest.java | 4 ++-- .../recordlayer/PlanGenerationStackTest.java | 10 +++++----- .../relational/recordlayer/QueryLoggingTest.java | 2 +- .../recordlayer/query/CaseSensitivityQueryTests.java | 12 ++++++------ .../recordlayer/query/TemporaryFunctionTests.java | 4 ++-- 7 files changed, 23 insertions(+), 18 deletions(-) diff --git a/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/Identifier.java b/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/Identifier.java index 93a5ec43ed..11266c57a4 100644 --- a/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/Identifier.java +++ b/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/Identifier.java @@ -29,6 +29,7 @@ import java.util.List; import java.util.Objects; import java.util.function.Function; +import java.util.stream.Collectors; @API(API.Status.EXPERIMENTAL) public class Identifier { @@ -59,7 +60,11 @@ public boolean isQualified() { @Override public String toString() { - return String.join(".", qualifier) + (qualifier.isEmpty() ? "" : ".") + name; + return qualifier.stream() + .map(q -> "\"" + q + "\"") + .collect(Collectors.joining(".")) + + (qualifier.isEmpty() ? "" : ".") + + "\"" + name + "\""; } @Nonnull diff --git a/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/SemanticAnalyzer.java b/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/SemanticAnalyzer.java index c551f202d7..fc6b0292dc 100644 --- a/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/SemanticAnalyzer.java +++ b/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/SemanticAnalyzer.java @@ -350,7 +350,7 @@ public Expression resolveIdentifier(@Nonnull Identifier identifier, return resolvedMaybe.get(); } } - Assert.failUnchecked(ErrorCode.UNDEFINED_COLUMN, String.format(Locale.ROOT, "Attempting to query non existing column '%s'", identifier)); + Assert.failUnchecked(ErrorCode.UNDEFINED_COLUMN, String.format(Locale.ROOT, "Attempting to query non existing column %s", identifier)); return null; // unreachable. } diff --git a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/api/ddl/IndexTest.java b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/api/ddl/IndexTest.java index c61ada2fc4..54d1f5039f 100644 --- a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/api/ddl/IndexTest.java +++ b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/api/ddl/IndexTest.java @@ -621,7 +621,7 @@ void failToCreateVersionIndexWithUnknownTable() throws Exception { "CREATE TABLE T1(col1 bigint, primary key(col1)) " + "CREATE INDEX mv1 AS SELECT t2.\"__ROW_VERSION\" FROM T1 AS t ORDER BY t2.\"__ROW_VERSION\" " + "WITH OPTIONS(store_row_versions=true)"; - shouldFailWith(stmt, ErrorCode.UNDEFINED_COLUMN, "Attempting to query non existing column 'T2.__ROW_VERSION'"); + shouldFailWith(stmt, ErrorCode.UNDEFINED_COLUMN, "Attempting to query non existing column \"T2\".\"__ROW_VERSION\""); } @Test @@ -680,7 +680,7 @@ void failToCreateVersionIndexWithAmbiguousSource() throws Exception { "CREATE TABLE T1(col1 bigint, a A Array, primary key(col1)) " + "CREATE INDEX mv1 AS SELECT X.col2, \"__ROW_VERSION\" FROM T1, (SELECT col2 FROM T1.A) X ORDER BY X.col2, \"__ROW_VERSION\" " + "WITH OPTIONS(store_row_versions=true)"; - shouldFailWith(stmt, ErrorCode.AMBIGUOUS_COLUMN, "Ambiguous reference '__ROW_VERSION'"); + shouldFailWith(stmt, ErrorCode.AMBIGUOUS_COLUMN, "Ambiguous reference '\"__ROW_VERSION\"'"); } @Test diff --git a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/PlanGenerationStackTest.java b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/PlanGenerationStackTest.java index 6314eb76bb..5c361dd091 100644 --- a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/PlanGenerationStackTest.java +++ b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/PlanGenerationStackTest.java @@ -87,7 +87,7 @@ public Stream provideArguments(final ExtensionContext conte Arguments.of(1, "select * from restaurant where rest_no > -10", null), Arguments.of(2, "sElEct * FrOm rESTaurant", null), Arguments.of(3, " select * from restaurant", null), - Arguments.of(4, "sElEct * FrOm \"RestaUrantRecord\"", "Unknown table RestaUrantRecord"), + Arguments.of(4, "sElEct * FrOm \"RestaUrantRecord\"", "Unknown table \"RestaUrantRecord\""), Arguments.of(5, "incorrect ", "incorrect[[]]^^^^^^^^^"), Arguments.of(6, "select * union restaurant", "select * union restaurant[[]] ^^^^^^^^^^"), Arguments.of(7, "select * from restaurant where rest_no > 10 ", null), @@ -97,7 +97,7 @@ public Stream provideArguments(final ExtensionContext conte Arguments.of(11, "select * from restaurant where rest_no <= 10 ", null), Arguments.of(12, "select * from restaurant where rest_no is null ", null), Arguments.of(13, "select * from restaurant where rest_no is not null ", null), - Arguments.of(14, "select * from restaurant where NON_EXISTING > 10 ", "Attempting to query non existing column 'NON_EXISTING'"), + Arguments.of(14, "select * from restaurant where NON_EXISTING > 10 ", "Attempting to query non existing column \"NON_EXISTING\""), Arguments.of(15, "select * from restaurant where rest_no > 'hello'", "unable to encapsulate comparison operation due to type mismatch(es)"), Arguments.of(16, "select * from restaurant where rest_no > 10 AND rest_no < 20", null), Arguments.of(17, "select * from restaurant where rest_no < 10 AND rest_no < 20", null), @@ -149,14 +149,14 @@ public Stream provideArguments(final ExtensionContext conte Arguments.of(63, "select * from restaurant USE INDEX (record_name_idx), USE INDEX (reviewer_name_idx) where rest_no > 10 ", "Unknown index(es) REVIEWER_NAME_IDX"), Arguments.of(64, "select * from restaurant with continuation", "syntax error[[]]select * from restaurant with continuation[[]] ^^"), Arguments.of(65, "select X.rest_no from (select rest_no from restaurant where 42 >= rest_no OR 42 > rest_no) X", null), - Arguments.of( 66, "select X.UNKNOWN from (select rest_no from restaurant where 42 >= rest_no OR 42 > rest_no) X", "Attempting to query non existing column 'X.UNKNOWN'"), + Arguments.of( 66, "select X.UNKNOWN from (select rest_no from restaurant where 42 >= rest_no OR 42 > rest_no) X", "Attempting to query non existing column \"X\".\"UNKNOWN\""), Arguments.of(67, "select X.rest_no from (select Y.rest_no from (select rest_no from restaurant where 42 >= rest_no OR 42 > rest_no) Y where 42 >= Y.rest_no OR 42 > Y.rest_no) X", null), Arguments.of(68, "select X.rating from restaurant AS Rec, (select rating from Rec.reviews) X", null), Arguments.of(69, "select COUNT(MAX(Y.rating)) FROM (select rest_no, X.rating from restaurant AS Rec, (select rating from Rec.reviews) X) as Y GROUP BY Y.rest_no", "unsupported nested aggregate(s) count(max_l"), - Arguments.of(70, "select rating from restaurant GROUP BY rest_no", "Attempting to query non existing column 'RATING'"), + Arguments.of(70, "select rating from restaurant GROUP BY rest_no", "Attempting to query non existing column \"RATING\""), // TODO understand why the query below cannot be planned //Arguments.of(71, "select rating + rest_no, MAX(rest_no) from (select rest_no, X.rating from restaurant AS Rec, (select rating from Rec.reviews) X) as Y GROUP BY rest_no, rating", null), - Arguments.of(72, "insert into restaurant_reviewer values (42, \"wrong\", null, null)", "Attempting to query non existing column 'wrong'"), + Arguments.of(72, "insert into restaurant_reviewer values (42, \"wrong\", null, null)", "Attempting to query non existing column \"wrong\""), Arguments.of(73, "with recursive c as (with c as (select * from restaurant) select * from c) select * from c", "ambiguous nested recursive CTE name"), Arguments.of(74, "with recursive c as (with c1 as (select * from restaurant) select * from c1) select * from c", null), Arguments.of(75, "with recursive c as (select * from t, c) select * from c", "recursive CTE does not contain non-recursive term"), diff --git a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/QueryLoggingTest.java b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/QueryLoggingTest.java index 00f82ff89d..0f39fa1856 100644 --- a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/QueryLoggingTest.java +++ b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/QueryLoggingTest.java @@ -364,7 +364,7 @@ void testLogException() { Assert.assertThrows(Exception.class, () -> statement.executeQuery("SELECT * FROM REST where rest_no = 0 OPTIONS (LOG QUERY)")); final var thrown = logAppender.getLastLogEvent().getThrown(); org.junit.jupiter.api.Assertions.assertNotNull(thrown); - Assertions.assertThat(thrown).hasMessage("Unknown table REST") + Assertions.assertThat(thrown).hasMessage("Unknown table \"REST\"") .hasNoCause(); } diff --git a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/CaseSensitivityQueryTests.java b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/CaseSensitivityQueryTests.java index 4dd9c73f72..28900af4af 100644 --- a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/CaseSensitivityQueryTests.java +++ b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/CaseSensitivityQueryTests.java @@ -63,14 +63,14 @@ void caseSensitiveConnectionTest() throws Exception { try (var statement = connection.createStatement()) { RelationalAssertions.assertThrowsSqlException(() -> statement.execute("select * from restaurant")) .hasErrorCode(ErrorCode.UNDEFINED_TABLE) - .hasMessageContaining("Unknown table RESTAURANT"); + .hasMessageContaining("Unknown table \"RESTAURANT\""); } connection.setOption(Options.Name.CASE_SENSITIVE_IDENTIFIERS, true); try (var statement = connection.createStatement()) { RelationalAssertions.assertThrowsSqlException(() -> statement.execute("select * from restaurant")) .hasErrorCode(ErrorCode.UNDEFINED_TABLE) - .hasMessageContaining("Unknown table restaurant"); + .hasMessageContaining("Unknown table \"restaurant\""); } } } @@ -96,14 +96,14 @@ void caseSensitiveConnectionTestCase2() throws Exception { try (var statement = connection.createStatement()) { RelationalAssertions.assertThrowsSqlException(() -> statement.execute("select * from \"restaurant\"")) .hasErrorCode(ErrorCode.UNDEFINED_TABLE) - .hasMessageContaining("Unknown table restaurant"); + .hasMessageContaining("Unknown table \"restaurant\""); } connection.setOption(Options.Name.CASE_SENSITIVE_IDENTIFIERS, true); try (var statement = connection.createStatement()) { RelationalAssertions.assertThrowsSqlException(() -> statement.execute("select * from ResTaurant")) .hasErrorCode(ErrorCode.UNDEFINED_TABLE) - .hasMessageContaining("Unknown table ResTaurant"); + .hasMessageContaining("Unknown table \"ResTaurant\""); } } } @@ -130,14 +130,14 @@ void caseSensitiveConnectionTestCase3(boolean isCaseSensitive) throws Exception try (var statement = connection.createStatement()) { RelationalAssertions.assertThrowsSqlException(() -> statement.execute("select * from restaurant")) .hasErrorCode(ErrorCode.UNDEFINED_TABLE) - .hasMessageContaining("Unknown table RESTAURANT"); + .hasMessageContaining("Unknown table \"RESTAURANT\""); } connection.setOption(Options.Name.CASE_SENSITIVE_IDENTIFIERS, true); try (var statement = connection.createStatement()) { RelationalAssertions.assertThrowsSqlException(() -> statement.execute("select * from restaurant")) .hasErrorCode(ErrorCode.UNDEFINED_TABLE) - .hasMessageContaining("Unknown table restaurant"); + .hasMessageContaining("Unknown table \"restaurant\""); } } } diff --git a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/TemporaryFunctionTests.java b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/TemporaryFunctionTests.java index a7cb8f5f95..3fc4e6b829 100644 --- a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/TemporaryFunctionTests.java +++ b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/TemporaryFunctionTests.java @@ -1023,7 +1023,7 @@ void attemptToCreateTemporaryFunctionWithDifferentCaseSensitivityOptionCase1() t RelationalAssertions.assertThrowsSqlException(() -> statement.execute("create temporary function sq1(in x bigint) " + "on commit drop function as select * from t1 where a < 40 + x ")) .hasErrorCode(ErrorCode.UNDEFINED_TABLE) - .hasMessageContaining("Unknown table T1"); + .hasMessageContaining("Unknown table \"T1\""); } connection.rollback(); } @@ -1045,7 +1045,7 @@ void attemptToCreateTemporaryFunctionWithDifferentCaseSensitivityOptionCase2() t RelationalAssertions.assertThrowsSqlException(() -> statement.execute("create temporary function sq1(in x bigint) " + "on commit drop function as select * from t1 where a < 40 + x ")) .hasErrorCode(ErrorCode.UNDEFINED_TABLE) - .hasMessageContaining("Unknown table t1"); + .hasMessageContaining("Unknown table \"t1\""); } connection.rollback(); }