Fix keyless-table numeric/temporal WHERE binding (port of tabularis#618)

[aesslinger]

Summary

Ports a parity fix from the built-in PostgreSQL driver, flagged by @debba on PR #3: TabularisDB/tabularis#618 fixed a bug where updating or deleting a row in a table with no primary key failed with:

ERROR: operator does not exist: numeric = text (SQLSTATE 42883)

Keyless tables identify rows by every column, so the WHERE predicate can target numeric/temporal columns. Their values reach the driver as JSON strings (numeric serializes as string to preserve arbitrary precision), and bind_pk_value bound them as plain TEXT — PostgreSQL has no implicit numeric = text (or timestamp = text) operator and rejects the statement.

This plugin has the exact same bug shape: bind_pg_string (used for SET binding on insert_record/update_record) already had the numeric/temporal coercion cascade, but bind_pk_value (used for WHERE predicates via build_pk_map_predicate — shared by update_record, delete_record, save_blob_to_file, and fetch_blob_as_data_url) never routed through it.

Fix

Factored the numeric/temporal coercion out of bind_pg_string into two shared functions — bind_pg_numeric_string / bind_pg_temporal_string, matching the builtin driver's own extracted-helper naming — and call them from both bind_pg_string and bind_pk_value. Unknown/non-numeric/non-temporal column types keep the existing shape heuristics unchanged.

TDD — full red→green trail

Per this project's standing discipline for parity fixes, wrote the tests before touching binding.rs:

1. Added 4 tests to src/binding_tests.rs's bind_pk_value_tests module, mirroring #618's own upstream test cases exactly:
   • numeric_column_string_value_casts_to_numeric — "1500.00" against a numeric column
   • double_precision_column_string_value_casts_to_double — "1.5" against a double precision column
   • numeric_column_unparsable_string_is_rejected — "abc" against a numeric column must error, not silently bind as text
   • timestamp_column_string_value_casts_through_text — "2024-05-01 10:30:00" against a timestamp column
2. Confirmed RED — cargo test bind_pk_value_tests before the fix:

   thread '...numeric_column_string_value_casts_to_numeric' panicked:
   assertion `left == right` failed
     left: "$2"
    right: "CAST($2 AS numeric)"
   
   thread '...double_precision_column_string_value_casts_to_double' panicked:
     left: "$1"
    right: "CAST($1 AS double precision)"
   
   thread '...numeric_column_unparsable_string_is_rejected' panicked:
   assertion failed: bind_pk_value(&json!("abc"), 1, Some("numeric")).is_err()
   
   thread '...timestamp_column_string_value_casts_through_text' panicked:
     left: "$3"
    right: "CAST($3 AS timestamp)"
   
   test result: FAILED. 6 passed; 4 failed
   

   All four failed exactly the way the missing coercion predicts: bound as a bare placeholder with no CAST, or (for the unparsable case) silently accepted instead of rejected.
3. Implemented the fix (the two shared functions + wiring bind_pk_value through them).
4. Confirmed GREEN — cargo test bind_pk_value_tests after: test result: ok. 10 passed; 0 failed. Full suite: cargo test --lib --bins → 82 passed; 0 failed (78 previous + 4 new), cargo clippy --all-targets -- -D warnings clean, cargo fmt --all -- --check clean, no Cargo.lock drift.

Functional verification against a live PostgreSQL instance

Unit tests prove the binding logic in isolation; this proves the fix closes the actual reported bug end-to-end, including a genuine before/after reproduction:

1. Reproduced the exact upstream bug first, against a real database (the local Podman pg-tabularis-test container, postgres:16):

   CREATE TABLE public.keyless_numeric_test (amount numeric, label text);
   INSERT INTO public.keyless_numeric_test VALUES (1500.00, 'row-a'), (250.50, 'row-b');

2. git stash'd the fix to rebuild the binary at main's current (pre-fix) state, then sent the exact request a real update_record call would make, identifying the row by its amount column (no PK exists):

   {"method":"update_record","params":{...},"table":"keyless_numeric_test","schema":"public","pk_map":{"amount":"1500.00"},"col_name":"label","new_val":"should-fail"}

   Result: {"error":{"code":-32603,"message":"Prepare failed: db error"}} — confirmed via psql that this is precisely numeric = text (ran SELECT 1500.00::numeric = '1500.00'::text directly, got the identical operator does not exist error PostgreSQL raises).
3. git stash pop'd the fix, rebuilt, and sent the identical request: {"result":1} — and SELECT * FROM keyless_numeric_test confirmed the row was actually updated (label changed to the new value), not just a fake success response.
4. Repeated for delete_record against the same table (amount = 250.50) — row correctly removed.
5. Repeated for a keyless table with a timestamp column (CREATE TABLE keyless_temporal_test (imported_at timestamp, label text)), identifying the row by imported_at — update_record succeeded and the row was correctly updated.
6. Cleaned up all scratch tables afterward.

Cross-repo parity gate

This change touches a shared binding path used by update_record, delete_record, save_blob_to_file, and fetch_blob_as_data_url — every method that identifies a row by primary key — so a regression here could be broad. Rebuilt the release binary and re-ran tabularis's existing, unmodified 82-test parity suite against it via POSTGRES_PLUGIN_BIN (same command used for every prior fix in this migration): 82/82 GREEN, confirming zero regression.

Test plan

• TDD: 4 new tests written first, confirmed RED, then GREEN after the fix (see full transcript above)
• cargo test --lib --bins: 82/82 unit tests pass (78 previous + 4 new)
• cargo clippy --all-targets -- -D warnings, cargo fmt --all -- --check: clean
• git diff --exit-code Cargo.lock: no drift
• Functional live-database reproduction: bug confirmed present pre-fix (exact 42883/"operator does not exist: numeric = text" failure), confirmed fixed post-fix, for both update_record and delete_record, across numeric and temporal keyless-identifying columns
• Cross-repo parity gate: 82/82 GREEN against tabularis's unmodified parity suite