Fix ER Diagram breakage for FKs to tables needing quoted names#883
Open
ChronicallyJD wants to merge 3 commits into
Open
Fix ER Diagram breakage for FKs to tables needing quoted names#883ChronicallyJD wants to merge 3 commits into
ChronicallyJD wants to merge 3 commits into
Conversation
ER Diagram generation failed with "Can not create edge with nonexistant
target" whenever a foreign key referenced a table whose name required
quoting (e.g. contained uppercase characters).
draw_graph() keyed graph nodes by the raw, unquoted table name from
QueryTables, but built foreign-key edges from QueryTablesForeignKeys,
which returns names via quote_ident(). For lowercase names quote_ident
is a no-op so the ids matched, but for names needing quotes the node id
("Testtable") and the edge target ('"Testtable"') diverged, so Cytoscape
could not find the target node and aborted rendering.
Key nodes by the quoted identifier (name_raw) so node ids match the
quoted FK edge endpoints, keeping the unquoted name as the display label,
and only create an edge when both endpoints reference an existing node.
This also fixes the is_fk/is_pk column-flag lookups, which used the same
quoted names.
Adds a regression test creating an uppercase-named table referenced by a
foreign key and asserting no edge points at a non-existent node.
Refs: commandprompt#882
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
draw_graph resolves the saved ERD layout by treating database_index as a Connection id, but the test fixture posts database_index 0 while its Connection row gets an autoincrement id, so the request failed with 400 'Connection matching query does not exist.' before reaching the graph assertions. Create a Connection row with id 0 inside the test so the lookup succeeds; the row is rolled back with the test transaction. Verified against a live PostgreSQL 16 on 127.0.0.1:5433: the test passes on this branch and fails on unpatched master at the quoted-node-id assertion. Refs: commandprompt#882 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keying graph nodes by name_raw regressed SQLite: its QueryTables returns name_raw as a quoted literal for every table, while its foreign-key metadata carries unquoted names, so no FK edge endpoint matched a node id and all edges (and is_fk/is_pk column flags) were silently dropped. Resolve edge endpoints by falling back to the unquoted display label when the reported name is not a node id itself, which keeps the graph correct for any backend regardless of how its FK metadata quotes names. Also fix html_id generation in the ERD tab: the sanitizing regex lacked the global flag, so only the first disallowed character run was replaced. A quoted node id (the very case fixed here) kept its trailing quote, and document.querySelector() in adjustSizes() then threw on the invalid selector, skipping the layout run. Replace every disallowed run and restrict the allowed set to CSS-selector-safe characters. Adds a SQLite ERD regression test exercising the label fallback; it needs no live database server. Verified together with the PostgreSQL regression test against a live PostgreSQL 16; the SQLite test fails without the view change. Refs: commandprompt#882 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
ER Diagram generation failed with "Can not create edge with nonexistant
target" whenever a foreign key referenced a table whose name required
quoting (e.g. contained uppercase characters).
Root cause:
draw_graph()keyed graph nodes by the raw, unquoted tablename from
QueryTables, but built foreign-key edges fromQueryTablesForeignKeys, which returns names viaquote_ident(). For namesneeding quotes the node id (
Testtable) and the edge target ("Testtable")diverged, so Cytoscape could not find the target node and aborted rendering.
Fix:
name_raw), keep the unquoted name asthe display label, and only create an edge when both endpoints reference an
existing node.
when the FK metadata name is not a node id itself — needed because SQLite
returns
name_rawquoted for every table while its FK pragma metadata isunquoted. This also fixes the
is_fk/is_pkcolumn-flag lookups.html_idgeneration in the ERD tab: the sanitizing regex lacked theglobal flag, so a quoted node id kept its trailing quote and
document.querySelector()inadjustSizes()threw on the invalidselector, skipping the layout run.
Testing: adds regression tests for PostgreSQL (live server) and SQLite
(temp file, no server needed). Both fail without their respective fixes and
pass with them.
Note: ERD layouts saved before this change store unquoted node ids, so
tables whose names require quoting lose their saved position once after
upgrading (they reappear via
new_nodes); positions of all other tables arepreserved.
Fixes #882