fix: backtick-quote non-bare object names everywhere SQL is generated#43
Merged
Conversation
Object names that aren't bare identifiers — dashes, dots, spaces, e.g. target_all.`part-00000-…-c000.snappy.parquet` — were emitted unquoted, so double-click (SELECT *), shift-click / node-click (SHOW CREATE), drag-to-insert, and autocomplete all produced SYNTAX_ERROR. Add pure helpers quoteIdent / qualifyIdent to core/format.js (backtick + escape only when a name isn't /^[A-Za-z_]\w*$/) and apply them at every site that turns an object name into SQL or editor text: - ui/schema.js — SELECT * FROM, SHOW CREATE (table + DATABASE), db/table/column drag identifiers, db double-click insert, column insert/cast. Keeps the raw `key` for internal identity (Sets, dbl-click tracking). - ui/explain-graph.js — schema-graph node click (split id on the first dot, quote each part). - core/completions.js — db/table/column completion `insert` values (label stays the bare name). Also fix core/schema-graph.js graph linking: dependencies_* / engine-arg / dict-source references carry the db separately, so join them unconditionally (new joinId) instead of via the dot heuristic — a dotted table name kept its db prefix instead of being mistaken for an already-qualified ref. The remaining heuristic (qualify) is now used only for MV-target / EXPLAIN-AST names that may genuinely be db-qualified, where a miss drops an edge rather than mis-linking. 866 tests pass; format.js + completions.js at 100%, gate holds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu
From the PR #43 self-review: - schema-graph.js: table-focus `center` still used the dot-heuristic qualify(), so dragging a dotted-name table (…snappy.parquet) onto the pane produced an EMPTY graph (center never matched a rowId node). Use joinId; add a regression test. Also simplify joinId to always emit `db.name` (guarantees a dot so node() splits correctly; was a `db ? … : name` ternary that could yield a dotless, mis-splittable id). - explain-graph.js: schemaClick no longer re-splits the node id — dagreLayout now passes db/name through (it already forwarded kind), so the UI quotes the parts the core already computed (no duplicated split convention). - completions.js: add a test asserting db/table/column `insert` is backtick- quoted for non-bare names (the quoting was previously unverified — bare names pass through unchanged, so a revert wouldn't have failed any test). - schema.js: hoist quoteIdent(db.db) to one local (was recomputed 3×). 868 tests pass; per-file coverage gate holds. Known follow-ups (not in this PR): the editor autocomplete path still can't match/qualify non-bare names (completionContext word/parent detection is [A-Za-z0-9_]-only), and qualify() remains a dot-heuristic for MV-target / EXPLAIN-AST names — both can drop/garble for dotted tables but the primary tree/graph vectors are correct. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
Object names that aren't bare identifiers — containing dashes, dots, or spaces — were interpolated into SQL unquoted, producing
SYNTAX_ERROR. Reported on github.demo with a Parquet-import table:Shift-click (SHOW CREATE) and double-click (SELECT *) both emitted
target_all.part-00000-…snappy.parquetwith no backticks →Code: 62 ... Syntax error.Audit — every place a name reached SQL/editor unquoted
schema.jsdouble-click tableSELECT * FROM <db.name>schema.jsshift-click tableSHOW CREATE <db.name>schema.jsshift-click DBSHOW CREATE DATABASE <db>schema.jsdouble-click DB<db>schema.jsdrag (db / table / column)IDENT_MIMEpayloadschema.jscolumn click<col>/<col>::Typeexplain-graph.jsschema-graph node clickSHOW CREATE <id>completions.jsinsertschema-graph.js(graph, not SQL).includes('.')heuristicFix
New pure helpers in
core/format.js:quoteIdent(name)— backtick-quote + escape (\,`) only whennameisn't/^[A-Za-z_]\w*$/, so ordinary SQL stays unquoted/readable.qualifyIdent(...parts)— quote each part, join with.(target_all.`a.b`).Applied at every site above (
labelkeeps the bare name for display;insert/SQL uses the quoted form). The schema tree's internalkey(Sets, double-click tracking) stays raw.For the graph:
dependencies_*/ engine-arg / dict-source references carry the db separately, so they now join unconditionally (joinId) instead of via the dot heuristic — a dotted table name keeps its db prefix. The heuristic (qualify) is now used only for MV-target /EXPLAIN ASTnames that may legitimately be db-qualified, where a miss drops an edge rather than mis-linking.Tests
format.test.js—quoteIdent/qualifyIdent(bare passthrough, backtick path, escaping, empty parts).schema.test.js— a…snappy.parquettable + anodd colcolumn: SELECT *, SHOW CREATE, drag, and column insert are all quoted.explain-graph.test.js— node click with a dotted name quotes the SHOW CREATE target.schema-graph.test.js— a dotted dependency keeps its db prefix.866 tests pass;
format.js+completions.jsat 100%, per-file coverage gate holds.Built and deployed to otel / antalya / github.demo (sha
4ea8412) for verification.🤖 Generated with Claude Code