feat: implement get_table_ddl RPC method for dump support - #119
Merged
Merged
Conversation
The host's dump_database now routes schema dumps through a DriverTrait get_table_ddl method instead of matching on the driver string, but this plugin had no handler for it, so dump-schema-structure failed with "method not implemented" for plugin-registered PostgreSQL connections. Reconstructs a CREATE TABLE statement from column metadata (name, type, NOT NULL, PRIMARY KEY), mirroring the built-in driver's implementation byte-for-byte. Extracts the column query get_columns already ran into a shared fetch_table_columns helper, and splits the DDL text assembly into a pure build_table_ddl function with unit test coverage.
aesslinger
added
enhancement
Version suggestionBased on this PR's title (
This is informational only — no tag or release is created automatically yet. |
The unit tests in metadata_tests.rs only exercise the pure build_table_ddl string builder with hand-fed column JSON — they never run the real query in fetch_table_columns against an actual server, so the new RPC handler had zero coverage of its live query path. Adds two cases to the existing live_db.rs harness (spawns the built plugin binary and talks JSON-RPC over stdio, same as tests/live_db.rs's other tests): a composite-PK table whose reconstructed DDL is asserted byte-for-byte, and a nonexistent table that must surface a JSON-RPC error. Ran locally against a real postgres:16 container matching the CI harness (POSTGRES_PLUGIN_BIN=target/debug/postgresql-plugin, port 54320) — both new cases pass, and the full 27-test live suite plus 336 unit tests are unaffected.
…et_table_ddl
A hard code-review pass against the built-in driver's own get_table_ddl
(tabularis/src-tauri/src/drivers/postgres/mod.rs) surfaced 13 candidate
issues. Most turned out to be byte-for-byte inherited from the built-in's
own implementation (enum/array type syntax, missing length/precision,
composite PK physical-vs-declared order, no defaults/comments, the
zero-column-table edge case) — per this repo's parity rule, those are
left alone rather than silently diverging from the driver being ported.
Two were genuine, fixable bugs specific to how this handler is wired up:
- Column names were spliced into the DDL as a bare `"{name}"` literal
with no escaping, unlike the schema/table name three lines below (which
already goes through the imported `quote_identifier`). A column named
`we"ird` (legal PostgreSQL) produced a truncated, corrupted statement.
Now reuses the same escaping helper for column names and PK columns.
- fetch_table_columns has no relkind/table_type filter. dump_database
normally lists tables via get_tables (BASE TABLE only), but an explicit
table selection bypasses that and can name a view directly — get_table_ddl
would silently return a fabricated, wrong CREATE TABLE instead of an
error. Adds a table_type check that rejects anything that isn't an
ordinary base table.
Also: strengthens the missing-table live test to assert the actual error
message (it previously only checked that some error came back), adds a
live test proving the new view-rejection guard against a real view, and
fixes the CHANGELOG's "## Unreleased" heading to match this file's
established "## [Unreleased]" (bracketed) convention — confirmed via
`git log -p` showing 11 prior bracketed headings vs. this one unbracketed
occurrence.
Verified against a real postgres:16 container (same fixture the CI
"Live PostgreSQL integration" job uses): 337 unit tests, 28 live tests
(1 pre-existing pgvector test correctly skipped), clippy -D warnings,
fmt --check, and markdownlint all clean.
aesslinger
force-pushed
the
feat/118-get-table-ddl
branch
from
September 27, 2026 14:19
266e30d to
e73c6d3
Compare
# Conflicts: # CHANGELOG.md # src/handlers/metadata_tests.rs # tests/live_db.rs
This was referenced Sep 29, 2026
aesslinger
added a commit
that referenced
this pull request
Sep 29, 2026
Ships the four pending PRs merged since 1.0.0-rc.4: get_table_ddl for dump-schema-structure (#119), get_schema_snapshot + the batch metadata RPCs for one-round-trip ER diagrams (#123, #121), session/transaction pinning across execute_query/execute_query_batch runs (#124), and the uuid 1.26.0 -> 1.26.1 patch bump (#108). Also documents the .tabularium id/name split that landed since rc.4 (#117); the CI workflow split (#116) is intentionally omitted per the rc.3/rc.4 convention of not changelogging CI-only changes. Verified: cargo build --release; 356 unit + 30 live-DB against the local pg-tabularis-test podman container; the cross-repo 83-test byte-for-byte parity suite (POSTGRES_PLUGIN_BIN against tabularis' src-tauri/tests/postgres_integration parity*) passes 83/83; cargo clippy --all-targets -D warnings; cargo fmt --all --check; npx markdownlint CHANGELOG.md -- all clean.
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.
Closes #118.
Problem
TabularisDB/tabularis#822 added a
get_table_ddlmethod to the host'sDriverTraitand dispatches it fromRpcDriveras a"get_table_ddl"RPC call, sodump_databasecan write a schema-preserving dump for plugin-registered PostgreSQL connections. TheRpcDriverside is wired, but this plugin had no handler for it, so dump-schema-structure failed with a "method not implemented" error.Fix
Adds a
get_table_ddlhandler insrc/handlers/metadata.rs, dispatched fromrpc.rs. Given{ params, table, schema }, it reconstructs a singleCREATE TABLEstatement from the table's columns — name, type,NOT NULL,PRIMARY KEY— mirroring the built-in driver'sget_table_ddl(src-tauri/src/drivers/postgres/mod.rs) byte-for-byte: no defaults, indexes, or FKs, matching its existing scope rather than expanding on it.fetch_table_columnsis extracted out ofget_columns(same query, samerow_to_table_columnmapping) so both handlers share one source of column metadata instead of duplicating the query.build_table_ddlis the pure DDL-text assembly, split out for unit testing without a live database — 7 cases insrc/handlers/metadata_tests.rscover single/composite primary keys, no primary key, an empty column set (table not found), quote-escaping in the schema-qualified name, and quote-escaping in a column name.crate::utils::identifiers::quote_identifier/qualifiedhelpers.Hardening from a follow-up hard review
A
/code-review maxpass against the diff, cross-checked against the built-in driver's actual source, surfaced 13 candidate issues. 11 turned out to be real bugs, but 9 of those are byte-for-byte inherited from the built-in's ownget_table_ddl/get_columns(enum/array type syntax, missingvarchar(N)/numeric(p,s)bounds, composite-PK physical-vs-declared column order, no defaults/SERIAL/comments, a zero-column-table edge case, domain types). Per this repo's parity rule ("don't improve on it silently; behavioral differences are regressions here, not fixes"), those are intentionally left alone rather than diverging from the driver being ported — fixing them would need a coordinated change (or a deliberate parity-break decision) across both repos, not a unilateral patch here.Two were fixed here, since they're specific to how this handler is wired up rather than inherited SQL-generation behavior:
"{name}"literal with no escaping, unlike the schema/table name a few lines below (which already goes throughquote_identifier). A column namedwe"ird(legal PostgreSQL) produced a truncated, corrupted statement. Now reuses the same escaping helper for column names and PK columns.dump_databasenormally lists tables viaget_tables(alreadyBASE TABLE-only), but an explicit table selection bypasses that and can name a view directly —get_table_ddlwould silently return a fabricated, wrongCREATE TABLEinstead of an error. Added atable_typecheck (ensure_base_table) that rejects anything that isn't an ordinary base table.Also fixed as part of the same pass: the missing-table live test now asserts the actual error message instead of just "some error came back," and the
CHANGELOG.mdheading was corrected from## Unreleasedto this file's established## [Unreleased](bracketed) convention — verified viagit log -p(11 prior bracketed headings vs. the one unbracketed occurrence this PR had introduced).Testing
cargo test --lib: 337 pass.cargo clippy --all-targets -- -D warnings,cargo fmt --all --check, andmarkdownlintall clean.postgres:16instance (the same fixturetests/live_db.rs's CI job uses): 28 live tests pass (1 pre-existing pgvector test correctly skipped), including 3 new cases forget_table_ddl— DDL reconstruction from a live composite-PK table, the missing-table error path, and the new view-rejection guard against a realCREATE VIEW.