fix(postgres): numeric/temporal WHERE binding for keyless tables + copy button in error modal - #618
Merged
Merged
Conversation
…edicates
Keyless tables identify rows by every column, so WHERE members can
target numeric or temporal columns whose values reach the driver as
JSON strings (numeric serializes as string to preserve precision).
These bound as plain TEXT, tripping SQLSTATE 42883
("operator does not exist: numeric = text") on update/delete.
Route known column types through the same numeric/temporal coercions
already used for SET binding.
Error messages shown in the alert modal can now be copied with one click, with a transient copied confirmation. Adds the common.copied key to all locales.
17 tasks
6 tasks
aesslinger
added a commit
to TabularisDB/tabularis-postgresql-plugin
that referenced
this pull request
Aug 11, 2026
…18) (#4) debba flagged (PR #3, comment) that the builtin PostgreSQL driver had a bug fixed upstream in TabularisDB/tabularis#618: updating or deleting a row in a table with no primary key failed with "operator does not exist: numeric = text" (SQLSTATE 42883). Keyless tables identify rows by every column, so the WHERE predicate can target numeric/temporal columns — whose values arrive as JSON strings (numeric serializes as string to preserve arbitrary precision) — but bind_pk_value bound them as plain TEXT with no coercion. This plugin shares the exact same bug shape: bind_pg_string (used for SET binding) already had the numeric/temporal coercion cascade, but bind_pk_value (used for WHERE predicates via build_pk_map_predicate — the shared path for update_record, delete_record, save_blob_to_file, and fetch_blob_as_data_url) never routed through it. TDD, per the project's standing instruction to keep following it for parity fixes: wrote 4 tests mirroring #618's own upstream test cases (numeric cast, double precision cast, unparsable-numeric rejection, timestamp cast-through-TEXT) against bind_pk_value first, confirmed all 4 failed (RED — bound as plain "$N" with no CAST, exactly the missing coercion). Fixed by factoring the numeric/temporal coercion out of bind_pg_string into two shared functions (bind_pg_numeric_string / bind_pg_temporal_string, matching the builtin's own extracted-helper naming) and calling them from both bind_pg_string and bind_pk_value. Confirmed GREEN: 82/82 unit tests (78 previous + 4 new). Functionally verified against a live PostgreSQL instance (the local Podman pg-tabularis-test container), not just unit tests: created a keyless table with a numeric column, confirmed update_record failed with the exact 42883 error on the pre-fix binary (git stash to rebuild main's current state), then confirmed the identical request succeeds and persists correctly on the fixed binary. Repeated for delete_record and a keyless temporal-column table (timestamp). Re-ran tabularis's unmodified 82-test cross-repo parity suite against the rebuilt release binary afterward — still 82/82 GREEN, zero regression.
aesslinger
added a commit
to TabularisDB/tabularis-postgresql-plugin
that referenced
this pull request
Aug 11, 2026
…18) (#4) debba flagged (PR #3, comment) that the builtin PostgreSQL driver had a bug fixed upstream in TabularisDB/tabularis#618: updating or deleting a row in a table with no primary key failed with "operator does not exist: numeric = text" (SQLSTATE 42883). Keyless tables identify rows by every column, so the WHERE predicate can target numeric/temporal columns — whose values arrive as JSON strings (numeric serializes as string to preserve arbitrary precision) — but bind_pk_value bound them as plain TEXT with no coercion. This plugin shares the exact same bug shape: bind_pg_string (used for SET binding) already had the numeric/temporal coercion cascade, but bind_pk_value (used for WHERE predicates via build_pk_map_predicate — the shared path for update_record, delete_record, save_blob_to_file, and fetch_blob_as_data_url) never routed through it. TDD, per the project's standing instruction to keep following it for parity fixes: wrote 4 tests mirroring #618's own upstream test cases (numeric cast, double precision cast, unparsable-numeric rejection, timestamp cast-through-TEXT) against bind_pk_value first, confirmed all 4 failed (RED — bound as plain "$N" with no CAST, exactly the missing coercion). Fixed by factoring the numeric/temporal coercion out of bind_pg_string into two shared functions (bind_pg_numeric_string / bind_pg_temporal_string, matching the builtin's own extracted-helper naming) and calling them from both bind_pg_string and bind_pk_value. Confirmed GREEN: 82/82 unit tests (78 previous + 4 new). Functionally verified against a live PostgreSQL instance (the local Podman pg-tabularis-test container), not just unit tests: created a keyless table with a numeric column, confirmed update_record failed with the exact 42883 error on the pre-fix binary (git stash to rebuild main's current state), then confirmed the identical request succeeds and persists correctly on the fixed binary. Repeated for delete_record and a keyless temporal-column table (timestamp). Re-ran tabularis's unmodified 82-test cross-repo parity suite against the rebuilt release binary afterward — still 82/82 GREEN, zero regression.
aesslinger
added a commit
to aesslinger/tabularis
that referenced
this pull request
Aug 12, 2026
…ruth TabularisDB/tabularis-postgresql-plugin has been extracted and is ahead of this copy (v1.0.0-beta.2 includes the TabularisDB#618 keyless-table WHERE-binding fix that never landed here), so this crate no longer serves as a valid parity baseline — the plugin repo's own CI (build/clippy/fmt/security audit/live-db integration) already covers what the in-tree build step and 82 parity tests were doing. Drops the plugin-build step and parity job from pg-integration.yml; keeps the baseline + golden tests, which remain meaningful for as long as the builtin driver exists. Cross-repo parity re-verification stays a manual check (POSTGRES_PLUGIN_BIN against a real release binary), per the plugin repo's own CLAUDE.md. Also fixes two test call sites (ddl_generation.rs, parity_ddl.rs) left broken by the upstream/main merge, which added a `params` argument to get_create_foreign_key_sql (TabularisDB#576), and regenerates 4 golden fixtures for the `is_generated` column-metadata field added upstream after these fixtures were captured. Full postgres_integration suite verified 180/180 green against a live PostgreSQL 16 instance.
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (14 files)
Reviewed by glm-5.2 · Input: 40.1K · Output: 7.1K · Cached: 311.8K |
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.
Problem
Updating a row in a table without a primary key failed with:
Keyless tables identify rows by every column, so the WHERE predicate can target numeric or temporal columns. Their values reach the driver as JSON strings (numeric serializes as string to preserve arbitrary precision), and
build_pk_predicatebound them as plain TEXT: PostgreSQL has no implicitnumeric = text(ortimestamp = text) operator and rejects the statement.Fix
In the string arm of
build_pk_predicate, when the column type is known, route the value through the same coercions already used for SET binding:bind_pg_numeric_string: parses and casts throughbigint/numeric/double precisionbind_pg_temporal_string:CAST($N AS timestamp/date/...)with the wire type pinned to TEXTUnknown column types keep the existing shape heuristics unchanged.
UI: copy button in the alert modal
The error modal now has a Copy button in the footer that copies the full message to the clipboard, with a transient "Copied!" confirmation (same pattern as the sidebar schema-error panel). Adds the
common.copiedkey to all 11 locales.Tests
build_pk_predicate(regression for the 42883 error)cargo test --lib drivers::postgres: 279 passedtsc --noEmit: clean