Skip to content

Commit c244ce6

Browse files
jogroganCopilot
andauthored
Handle dotted table names in quoted identifiers (#248)
A quoted identifier segment may legitimately contain a dot, e.g. the Kafka topic "KAFKA"."my.event". Several code paths took a bare identifier string and naively split("\\."), shredding "my.event" into "my" + "event" and breaking !describe / !graph / !resolve and graph / custom-resource lookups. Add IdentifierUtils.parseIdentifier, which splits unquoted input directly on '.' (exact, since an unquoted segment cannot contain a dot — this also covers the unquoted hyphenated names the CLI commands accept, e.g. LOGICAL.testevent-graph) and uses the SQL parser only for quoted input so a dot inside a quoted segment is preserved. Malformed quoted identifiers now error rather than silently mis-splitting. Wire it into GraphService.resolve, the !resolve/!describe CLI and quidem paths, and PipelineGraphBuilder. K8s object names already permit dots, so canonicalization is left unchanged (backwards compatible with existing dotted resource names). Add unit tests for IdentifierUtils, dot-preservation tests for K8sUtils.canonicalizeName, a GraphService test, and a Kafka integration script that creates/describes/drops a topic whose name contains a dot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d0387ae commit c244ce6

10 files changed

Lines changed: 265 additions & 18 deletions

File tree

hoptimator-cli/src/main/java/sqlline/HoptimatorAppConfig.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import com.linkedin.hoptimator.jdbc.ResolvedTable;
1212
import com.linkedin.hoptimator.jdbc.ddl.SqlCreateMaterializedView;
1313
import com.linkedin.hoptimator.util.DeploymentService;
14+
import com.linkedin.hoptimator.util.IdentifierUtils;
1415
import com.linkedin.hoptimator.util.planner.PipelineRel;
1516
import org.apache.calcite.plan.RelOptTable;
1617
import org.apache.calcite.rel.RelRoot;
@@ -24,7 +25,6 @@
2425

2526
import java.nio.charset.StandardCharsets;
2627
import java.util.ArrayList;
27-
import java.util.Arrays;
2828
import java.util.Collection;
2929
import java.util.Collections;
3030
import java.util.List;
@@ -205,7 +205,7 @@ public void execute(String line, DispatchCallback dispatchCallback) {
205205
dispatchCallback.setToFailure();
206206
return;
207207
}
208-
List<String> tablePath = Arrays.asList(split[1].split("\\."));
208+
List<String> tablePath = IdentifierUtils.parseIdentifier(split[1]);
209209
HoptimatorConnection conn = (HoptimatorConnection) sqlline.getConnection();
210210
try {
211211
ResolvedTable resolved = conn.resolve(tablePath, Collections.emptyMap());

hoptimator-jdbc/src/main/java/com/linkedin/hoptimator/jdbc/GraphService.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.linkedin.hoptimator.jdbc;
22

33
import java.sql.SQLException;
4-
import java.util.Arrays;
54
import java.util.ArrayList;
65
import java.util.List;
76
import java.util.ServiceLoader;
@@ -15,6 +14,7 @@
1514
import com.linkedin.hoptimator.graph.GraphRenderer;
1615
import com.linkedin.hoptimator.graph.GraphTarget;
1716
import com.linkedin.hoptimator.graph.PipelineGraph;
17+
import com.linkedin.hoptimator.util.IdentifierUtils;
1818
import com.linkedin.hoptimator.util.planner.HoptimatorJdbcSchema;
1919
import org.apache.calcite.util.Util;
2020

@@ -76,7 +76,7 @@ public static PipelineGraph buildGraph(String identifier, int depth, HoptimatorC
7676
* {@code HoptimatorJdbcSchema} lazily walks its downstream connection to find the marker.
7777
*/
7878
static GraphTarget resolve(String identifier, HoptimatorConnection connection) throws SQLException {
79-
List<String> fullPath = Arrays.asList(identifier.split("\\."));
79+
List<String> fullPath = IdentifierUtils.parseIdentifier(identifier);
8080
SchemaPlus schema = connection.calciteConnection().getRootSchema();
8181
List<String> schemaPath = Util.skipLast(fullPath);
8282
for (String segment : schemaPath) {

hoptimator-jdbc/src/test/java/com/linkedin/hoptimator/jdbc/GraphServiceTest.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,21 @@ void resolveTwoLevelResourceFindsDatabaseCrName() throws SQLException {
8181
assertEquals(Arrays.asList("ADS", "AD_CLICKS"), out.path());
8282
}
8383

84+
@Test
85+
void resolveQuotedDottedTableNameTreatsQuotedSegmentAsOneIdentity() throws SQLException {
86+
// User typed "VENICE"."my.table". The dot inside the quoted segment is part of the table
87+
// name, NOT a path separator — so this must resolve to schema VENICE, table "my.table".
88+
// A naive identifier.split("\\.") shreds this into [VENICE, my, table] and fails.
89+
SchemaPlus venice = schemaWithDatabaseAndTable("venice-database", "my.table", plainTable());
90+
stubRootSchema(schemaWithSubs("VENICE", venice));
91+
92+
GraphTarget.Resource out = (GraphTarget.Resource) GraphService.resolve(
93+
"\"VENICE\".\"my.table\"", connection);
94+
95+
assertEquals("venice-database", out.database());
96+
assertEquals(Arrays.asList("VENICE", "my.table"), out.path());
97+
}
98+
8499
@Test
85100
void resolveThreeLevelResourceWalksCatalogAndSchema() throws SQLException {
86101
// User typed MYSQL.testdb.orders. MYSQL is the catalog sub-schema; testdb is the Database;

hoptimator-jdbc/src/testFixtures/java/com/linkedin/hoptimator/jdbc/QuidemTestBase.java

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
import com.linkedin.hoptimator.graph.PipelineGraph;
1515
import com.linkedin.hoptimator.graph.mermaid.MermaidRenderer;
16+
import com.linkedin.hoptimator.util.IdentifierUtils;
1617

1718
import java.io.File;
1819
import java.io.FileReader;
@@ -98,9 +99,10 @@ public void execute(Context context, boolean execute) throws Exception {
9899
throw new IllegalArgumentException("Usage: !describe \"SCHEMA\".\"TABLE\" or \"CATALOG\".\"SCHEMA\".\"TABLE\"");
99100
}
100101

101-
String tablePath = parts[1].trim().replaceAll("\"", "");
102-
String[] pathParts = tablePath.split("\\.");
103-
if (pathParts.length < 2) {
102+
// Parse quote-aware so a dot inside a quoted segment (e.g. "KAFKA"."my.event") stays
103+
// part of the name rather than being treated as a path separator.
104+
List<String> pathParts = IdentifierUtils.parseIdentifier(parts[1].trim());
105+
if (pathParts.size() < 2) {
104106
throw new IllegalArgumentException("Table path must be at least SCHEMA.TABLE");
105107
}
106108

@@ -109,24 +111,24 @@ public void execute(Context context, boolean execute) throws Exception {
109111
SchemaPlus schema;
110112
String tableName;
111113

112-
if (pathParts.length == 3) {
114+
if (pathParts.size() == 3) {
113115
// 3-level path: CATALOG.SCHEMA.TABLE (e.g., MySQL)
114-
SchemaPlus catalog = rootSchema.subSchemas().get(pathParts[0]);
116+
SchemaPlus catalog = rootSchema.subSchemas().get(pathParts.get(0));
115117
if (catalog == null) {
116-
throw new IllegalArgumentException("Catalog not found: " + pathParts[0]);
118+
throw new IllegalArgumentException("Catalog not found: " + pathParts.get(0));
117119
}
118-
schema = catalog.subSchemas().get(pathParts[1]);
120+
schema = catalog.subSchemas().get(pathParts.get(1));
119121
if (schema == null) {
120-
throw new IllegalArgumentException("Schema not found: " + pathParts[1] + " in catalog " + pathParts[0]);
122+
throw new IllegalArgumentException("Schema not found: " + pathParts.get(1) + " in catalog " + pathParts.get(0));
121123
}
122-
tableName = pathParts[2];
124+
tableName = pathParts.get(2);
123125
} else {
124126
// 2-level path: SCHEMA.TABLE (e.g., Kafka, Venice)
125-
schema = rootSchema.subSchemas().get(pathParts[0]);
127+
schema = rootSchema.subSchemas().get(pathParts.get(0));
126128
if (schema == null) {
127-
throw new IllegalArgumentException("Schema not found: " + pathParts[0]);
129+
throw new IllegalArgumentException("Schema not found: " + pathParts.get(0));
128130
}
129-
tableName = pathParts[pathParts.length - 1];
131+
tableName = pathParts.get(pathParts.size() - 1);
130132
}
131133

132134
Table table = schema.tables().get(tableName);

hoptimator-k8s/src/main/java/com/linkedin/hoptimator/k8s/PipelineGraphBuilder.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.linkedin.hoptimator.graph.GraphEdge;
2424
import com.linkedin.hoptimator.graph.GraphNode;
2525
import com.linkedin.hoptimator.graph.PipelineGraph;
26+
import com.linkedin.hoptimator.util.IdentifierUtils;
2627
import com.linkedin.hoptimator.k8s.models.V1alpha1LogicalTable;
2728
import com.linkedin.hoptimator.k8s.models.V1alpha1LogicalTableList;
2829
import com.linkedin.hoptimator.k8s.models.V1alpha1LogicalTableSpec;
@@ -84,7 +85,7 @@ public PipelineGraph forView(String name) throws SQLException {
8485
// stored under a canonicalized name (lowercase, {@code _} stripped, {@code $} → {@code -},
8586
// dot-separated parts joined with {@code -}). Canonicalization is idempotent, so passing the
8687
// already-canonical custom resource name still works.
87-
String crName = K8sUtils.canonicalizeName(Arrays.asList(name.split("\\.")));
88+
String crName = K8sUtils.canonicalizeName(IdentifierUtils.parseIdentifier(name));
8889
V1alpha1View view = viewApi.get(crName);
8990
if (view.getSpec() == null) {
9091
throw new SQLException("view " + crName + " not found");
@@ -118,7 +119,7 @@ public PipelineGraph forView(String name) throws SQLException {
118119
public PipelineGraph forLogicalTable(String name) throws SQLException {
119120
// Same canonicalization as forView — accept SQL-side identifiers and resolve to the
120121
// canonicalized custom resource name.
121-
String crName = K8sUtils.canonicalizeName(Arrays.asList(name.split("\\.")));
122+
String crName = K8sUtils.canonicalizeName(IdentifierUtils.parseIdentifier(name));
122123
V1alpha1LogicalTable lt = logicalTableApi.get(crName);
123124
Map<String, String> tierMap = tierMap(lt.getSpec());
124125
GraphNode.LogicalTable root = new GraphNode.LogicalTable(crName, tierMap);

hoptimator-k8s/src/test/java/com/linkedin/hoptimator/k8s/K8sUtilsTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,20 @@ void canonicalizeNameReplacesUnderscoresAndDollarSigns() {
4141
assertEquals("my-table", K8sUtils.canonicalizeName("my_$table"));
4242
}
4343

44+
@Test
45+
void canonicalizeNamePreservesDots() {
46+
// Dots are intentionally NOT rewritten: K8s object names here are DNS-1123 subdomains, which
47+
// permit dots, and existing deployed resources already carry dotted names. Rewriting dots
48+
// would change the derived name and make the operator create duplicates instead of updating
49+
// the pre-existing resource. Keeping dots keeps create-time and lookup-time names in agreement.
50+
assertEquals("my.event", K8sUtils.canonicalizeName("my.event"));
51+
}
52+
53+
@Test
54+
void canonicalizeNameWithDottedTableAndDatabasePreservesDot() {
55+
assertEquals("kafka-database-my.event", K8sUtils.canonicalizeName("kafka-database", "my.event"));
56+
}
57+
4458
@Test
4559
void canonicalizeNameWithDatabaseAndTable() {
4660
assertEquals("mydb-mytable", K8sUtils.canonicalizeName("myDB", "myTable"));

hoptimator-kafka/src/test/java/com/linkedin/hoptimator/kafka/TestSqlScripts.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,9 @@ public void kafkaDdlScriptBeamJob() throws Exception {
2222
public void kafkaDdlCreateTableScript() throws Exception {
2323
run("kafka-ddl-create-table.id");
2424
}
25+
26+
@Test
27+
public void kafkaDdlDottedTableScript() throws Exception {
28+
run("kafka-ddl-dotted.id");
29+
}
2530
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
!set outputformat mysql
2+
!use k8s
3+
4+
# ─────────────────────────────────────────────────────────────────────────────
5+
# A Kafka topic whose *name* contains a dot. Quoted as "KAFKA"."my.event", the
6+
# dot is part of the topic identity, NOT a catalog/schema separator. Creating,
7+
# describing, and dropping it must all treat "my.event" as a single identifier.
8+
# Before the quote-aware identifier fix, !describe split this into a bogus
9+
# KAFKA.my.event three-level path and failed with "Schema not found: my".
10+
# ─────────────────────────────────────────────────────────────────────────────
11+
12+
create or replace table "KAFKA"."my.event" ("KEY" VARCHAR, "VALUE" BINARY) WITH ("kafka.partitions" '1');
13+
(0 rows modified)
14+
15+
!update
16+
17+
+------------+-----------+------------+------------+
18+
| columnName | typeName | columnSize | isNullable |
19+
+------------+-----------+------------+------------+
20+
| KEY | VARCHAR | null | YES |
21+
| VALUE | BINARY(1) | 1 | YES |
22+
+------------+-----------+------------+------------+
23+
(2 rows)
24+
25+
!describe "KAFKA"."my.event"
26+
27+
drop table "KAFKA"."my.event";
28+
(0 rows modified)
29+
30+
!update
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package com.linkedin.hoptimator.util;
2+
3+
import java.util.ArrayList;
4+
import java.util.Arrays;
5+
import java.util.List;
6+
7+
import org.apache.calcite.avatica.util.Casing;
8+
import org.apache.calcite.avatica.util.Quoting;
9+
import org.apache.calcite.sql.SqlIdentifier;
10+
import org.apache.calcite.sql.SqlNode;
11+
import org.apache.calcite.sql.parser.SqlParser;
12+
import org.apache.calcite.sql.parser.SqlParseException;
13+
import org.apache.calcite.sql.validate.SqlConformanceEnum;
14+
15+
16+
/** Helpers for turning user-typed SQL identifier strings into their component parts. */
17+
public final class IdentifierUtils {
18+
19+
private static final SqlParser.Config PARSER_CONFIG = SqlParser.config()
20+
.withQuoting(Quoting.DOUBLE_QUOTE)
21+
// Preserve case exactly as typed — Hoptimator schema/table names are case-sensitive.
22+
.withUnquotedCasing(Casing.UNCHANGED)
23+
.withQuotedCasing(Casing.UNCHANGED)
24+
.withConformance(SqlConformanceEnum.BABEL);
25+
26+
private IdentifierUtils() {
27+
}
28+
29+
/**
30+
* Splits a possibly-quoted, dot-separated SQL identifier into its component parts, honoring
31+
* double-quote quoting so that a dot <em>inside</em> a quoted segment is treated as part of the
32+
* name rather than a path separator. For example {@code "VENICE"."my.table"} yields
33+
* {@code [VENICE, my.table]}, not {@code [VENICE, my, table]}.
34+
*
35+
* <p>Hoptimator accepts bare identifier strings on several paths (the {@code !graph}/
36+
* {@code !resolve}/{@code !describe} CLI commands, graph resolution, custom-resource lookups).
37+
* Those paths must not naively {@code split("\\.")}, or they corrupt names that legitimately
38+
* contain a dot.
39+
*
40+
* <p>The work splits cleanly by quoting:
41+
* <ul>
42+
* <li><b>Unquoted input</b> ({@code no '"'}) is split directly on {@code .}. This is exact: an
43+
* unquoted segment cannot itself contain a dot, so there is nothing for the SQL parser to
44+
* disambiguate. It also covers names that are <em>not</em> valid standalone SQL identifiers
45+
* but that Hoptimator accepts on its bare-string CLI commands — e.g. the unquoted
46+
* hyphenated {@code LOGICAL.testevent-graph} (which the SQL grammar would read as the
47+
* subtraction {@code testevent - graph}).</li>
48+
* <li><b>Quoted input</b> is the <em>only</em> reason this helper exists: it runs the SQL
49+
* parser so a dot inside a quoted segment ({@code "my.table"}) stays intact. If the quoted
50+
* input does not parse to a single identifier — e.g. a malformed or mixed form like
51+
* {@code "KAFKA".my-topic}, where an unquoted hyphenated segment is illegal — it throws
52+
* rather than silently mis-splitting.</li>
53+
* </ul>
54+
*
55+
* @throws IllegalArgumentException if {@code identifier} is quoted but is not a well-formed
56+
* dotted identifier.
57+
*/
58+
public static List<String> parseIdentifier(String identifier) {
59+
if (identifier == null) {
60+
return new ArrayList<>();
61+
}
62+
// Unquoted: a plain dot-split is exact (no segment can contain a dot). This is also the path
63+
// for the unquoted hyphenated names Hoptimator's CLI commands accept but the SQL grammar does
64+
// not, so it must NOT go through the parser.
65+
if (identifier.indexOf('"') < 0) {
66+
return new ArrayList<>(Arrays.asList(identifier.split("\\.")));
67+
}
68+
// Quoted: use the parser so a dot inside a quoted segment is preserved.
69+
try {
70+
SqlNode node = SqlParser.create(identifier, PARSER_CONFIG).parseExpression();
71+
if (node instanceof SqlIdentifier) {
72+
SqlIdentifier id = (SqlIdentifier) node;
73+
if (!id.isStar() && !id.names.isEmpty()) {
74+
return new ArrayList<>(id.names);
75+
}
76+
}
77+
// Parsed, but not a plain identifier (e.g. "KAFKA".my-topic reads as a subtraction).
78+
throw new IllegalArgumentException("Not a well-formed quoted table identifier: " + identifier);
79+
} catch (SqlParseException e) {
80+
throw new IllegalArgumentException("Not a well-formed quoted table identifier: " + identifier, e);
81+
}
82+
}
83+
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package com.linkedin.hoptimator.util;
2+
3+
import java.util.List;
4+
5+
import org.junit.jupiter.api.Test;
6+
7+
import static org.assertj.core.api.Assertions.assertThat;
8+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
9+
10+
11+
class IdentifierUtilsTest {
12+
13+
@Test
14+
void quotedDottedSegmentStaysWhole() {
15+
// The dot inside "my.table" is part of the name, not a separator.
16+
assertThat(IdentifierUtils.parseIdentifier("\"VENICE\".\"my.table\""))
17+
.containsExactly("VENICE", "my.table");
18+
}
19+
20+
@Test
21+
void quotedDottedTopicStaysWhole() {
22+
assertThat(IdentifierUtils.parseIdentifier("\"KAFKA\".\"my.event\""))
23+
.containsExactly("KAFKA", "my.event");
24+
}
25+
26+
@Test
27+
void unquotedIdentifierSplitsOnDots() {
28+
assertThat(IdentifierUtils.parseIdentifier("ADS.AD_CLICKS"))
29+
.containsExactly("ADS", "AD_CLICKS");
30+
}
31+
32+
@Test
33+
void threeLevelUnquotedIdentifierSplits() {
34+
assertThat(IdentifierUtils.parseIdentifier("MYSQL.testdb.orders"))
35+
.containsExactly("MYSQL", "testdb", "orders");
36+
}
37+
38+
@Test
39+
void unquotedHyphenatedNameSplitsOnDots() {
40+
// Calcite would read "LOGICAL.testevent-graph" as arithmetic (subtraction), but unquoted input
41+
// never reaches the parser: a plain dot-split is exact because an unquoted segment cannot
42+
// contain a dot. This is the form the !graph/!resolve/!describe CLI commands accept.
43+
assertThat(IdentifierUtils.parseIdentifier("LOGICAL.testevent-graph"))
44+
.containsExactly("LOGICAL", "testevent-graph");
45+
}
46+
47+
@Test
48+
void malformedQuotedIdentifierThrows() {
49+
// Quoted input must be a well-formed identifier. A mixed form — quoted schema with an unquoted
50+
// hyphenated segment — is illegal (the SQL grammar reads it as subtraction), so surface it
51+
// rather than silently mis-splitting.
52+
assertThatThrownBy(() -> IdentifierUtils.parseIdentifier("\"KAFKA\".my-topic"))
53+
.isInstanceOf(IllegalArgumentException.class)
54+
.hasMessageContaining("\"KAFKA\".my-topic");
55+
}
56+
57+
@Test
58+
void unterminatedQuoteThrows() {
59+
assertThatThrownBy(() -> IdentifierUtils.parseIdentifier("\"KAFKA\".\"my.event"))
60+
.isInstanceOf(IllegalArgumentException.class);
61+
}
62+
63+
@Test
64+
void quotedHyphenDollarNamePreservesSpecialChars() {
65+
assertThat(IdentifierUtils.parseIdentifier("VENICE.\"test-store$insert-partial\""))
66+
.containsExactly("VENICE", "test-store$insert-partial");
67+
}
68+
69+
@Test
70+
void casePreservedForUnquotedAndQuoted() {
71+
assertThat(IdentifierUtils.parseIdentifier("profile.Members"))
72+
.containsExactly("profile", "Members");
73+
}
74+
75+
@Test
76+
void singleSegmentReturnsOneElement() {
77+
assertThat(IdentifierUtils.parseIdentifier("audience")).containsExactly("audience");
78+
}
79+
80+
@Test
81+
void singleQuotedSegmentWithDotStaysWhole() {
82+
assertThat(IdentifierUtils.parseIdentifier("\"my.event\"")).containsExactly("my.event");
83+
}
84+
85+
@Test
86+
void escapedQuotesInsideSegmentAreUnescaped() {
87+
// "a""b" is the SQL-quoted form of the identifier a"b.
88+
assertThat(IdentifierUtils.parseIdentifier("\"a\"\"b\".\"c.d\""))
89+
.containsExactly("a\"b", "c.d");
90+
}
91+
92+
@Test
93+
void nullReturnsEmptyList() {
94+
List<String> parts = IdentifierUtils.parseIdentifier(null);
95+
assertThat(parts).isEmpty();
96+
}
97+
}

0 commit comments

Comments
 (0)