From 98b7c21946fa58555d37a746caebb60711955efa Mon Sep 17 00:00:00 2001 From: Adam J Esslinger Date: Tue, 11 Aug 2026 08:19:36 -0400 Subject: [PATCH] Fix keyless-table numeric/temporal WHERE binding (port of tabularis#618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/binding.rs | 149 ++++++++++++++++++++++++++++++------------- src/binding_tests.rs | 43 +++++++++++++ 2 files changed, 148 insertions(+), 44 deletions(-) diff --git a/src/binding.rs b/src/binding.rs index ce482ea..dec1738 100644 --- a/src/binding.rs +++ b/src/binding.rs @@ -170,55 +170,15 @@ fn bind_pg_string( // 5. Numeric column if let Some(bt) = base_type { - match bt { - "SMALLINT" | "INTEGER" | "BIGINT" | "INT2" | "INT4" | "INT8" | "SERIAL" - | "BIGSERIAL" => { - let i: i64 = s.parse().map_err(|_| { - format!("Cannot bind '{}' as integer for target type {}", s, bt) - })?; - return Ok(BoundValue { - sql: format!("CAST(${} AS bigint)", placeholder_idx), - param: Some((Box::new(i), Type::INT8)), - }); - } - "NUMERIC" | "DECIMAL" => { - let d: Decimal = s.parse().map_err(|_| { - format!("Cannot bind '{}' as numeric for target type {}", s, bt) - })?; - return Ok(BoundValue { - sql: format!("CAST(${} AS numeric)", placeholder_idx), - param: Some((Box::new(d), Type::NUMERIC)), - }); - } - "REAL" | "DOUBLE PRECISION" | "FLOAT4" | "FLOAT8" => { - let f: f64 = s - .parse() - .map_err(|_| format!("Cannot bind '{}' as float for target type {}", s, bt))?; - return Ok(BoundValue { - sql: format!("CAST(${} AS double precision)", placeholder_idx), - param: Some((Box::new(f), Type::FLOAT8)), - }); - } - _ => {} + if let Some(bound) = bind_pg_numeric_string(s, bt, placeholder_idx) { + return bound; } } // 6. Temporal column if let Some(bt) = base_type { - let cast_target = match bt { - "TIMESTAMP" | "TIMESTAMP WITHOUT TIME ZONE" => Some("timestamp"), - "TIMESTAMPTZ" | "TIMESTAMP WITH TIME ZONE" => Some("timestamptz"), - "DATE" => Some("date"), - "TIME" | "TIME WITHOUT TIME ZONE" => Some("time"), - "TIMETZ" | "TIME WITH TIME ZONE" => Some("timetz"), - "INTERVAL" => Some("interval"), - _ => None, - }; - if let Some(target) = cast_target { - return Ok(BoundValue { - sql: format!("CAST(${} AS {})", placeholder_idx, target), - param: Some((Box::new(s.to_string()), Type::TEXT)), - }); + if let Some(bound) = bind_pg_temporal_string(s, bt, placeholder_idx) { + return bound; } } @@ -301,6 +261,91 @@ fn json_array_to_pg_literal(arr: &[Value]) -> Result { Ok(format!("ARRAY[{}]", parts.join(", "))) } +/// Coerce a string value into a numeric PostgreSQL column (`smallint` +/// through `bigint`, `numeric`/`decimal`, `real`/`double precision`). +/// Returns `None` if the column is not numeric, so callers can fall through +/// to the next coercion path. Shared by SET binding (`bind_pg_string`) and +/// WHERE-predicate binding (`bind_pk_value`) — factored out to match the +/// builtin driver's `bind_pg_numeric_string`. +fn bind_pg_numeric_string( + s: &str, + base_type: &str, + placeholder_idx: usize, +) -> Option> { + match base_type { + "SMALLINT" | "INTEGER" | "BIGINT" | "INT2" | "INT4" | "INT8" | "SERIAL" | "BIGSERIAL" => { + Some(s.parse::().map_or_else( + |_| { + Err(format!( + "Cannot bind '{}' as integer for target type {}", + s, base_type + )) + }, + |i| { + Ok(BoundValue { + sql: format!("CAST(${} AS bigint)", placeholder_idx), + param: Some((Box::new(i), Type::INT8)), + }) + }, + )) + } + "NUMERIC" | "DECIMAL" => Some(s.parse::().map_or_else( + |_| { + Err(format!( + "Cannot bind '{}' as numeric for target type {}", + s, base_type + )) + }, + |d| { + Ok(BoundValue { + sql: format!("CAST(${} AS numeric)", placeholder_idx), + param: Some((Box::new(d), Type::NUMERIC)), + }) + }, + )), + "REAL" | "DOUBLE PRECISION" | "FLOAT4" | "FLOAT8" => Some(s.parse::().map_or_else( + |_| { + Err(format!( + "Cannot bind '{}' as float for target type {}", + s, base_type + )) + }, + |f| { + Ok(BoundValue { + sql: format!("CAST(${} AS double precision)", placeholder_idx), + param: Some((Box::new(f), Type::FLOAT8)), + }) + }, + )), + _ => None, + } +} + +/// Coerce a string value into a temporal PostgreSQL column (`timestamp`, +/// `timestamptz`, `date`, `time`, `timetz`, `interval`). Returns `None` if +/// the column is not temporal. Shared by SET binding and WHERE-predicate +/// binding — factored out to match the builtin driver's +/// `bind_pg_temporal_string`. +fn bind_pg_temporal_string( + s: &str, + base_type: &str, + placeholder_idx: usize, +) -> Option> { + let cast_target = match base_type { + "TIMESTAMP" | "TIMESTAMP WITHOUT TIME ZONE" => "timestamp", + "TIMESTAMPTZ" | "TIMESTAMP WITH TIME ZONE" => "timestamptz", + "DATE" => "date", + "TIME" | "TIME WITHOUT TIME ZONE" => "time", + "TIMETZ" | "TIME WITH TIME ZONE" => "timetz", + "INTERVAL" => "interval", + _ => return None, + }; + Some(Ok(BoundValue { + sql: format!("CAST(${} AS {})", placeholder_idx, cast_target), + param: Some((Box::new(s.to_string()), Type::TEXT)), + })) +} + /// Bind a WHERE-clause value from a PK map entry. Returns the SQL fragment /// (may include a CAST) plus the typed parameter — stricter than /// `bind_pg_value` for strings: UUID/integer string coercion is only applied @@ -340,6 +385,22 @@ pub fn bind_pk_value( } } + // 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). Route them through the same + // coercions as SET binding — a plain TEXT bind trips SQLSTATE + // 42883, e.g. "operator does not exist: numeric = text". + // Ported from the builtin driver's parity fix + // (TabularisDB/tabularis#618). + if let Some(bt) = base_type.as_deref() { + if let Some(bound) = bind_pg_numeric_string(s, bt, placeholder_idx) + .or_else(|| bind_pg_temporal_string(s, bt, placeholder_idx)) + { + return bound; + } + } + let is_int_type = base_type.as_deref().is_none_or(|t| { matches!( t, diff --git a/src/binding_tests.rs b/src/binding_tests.rs index 6deca50..bdc6314 100644 --- a/src/binding_tests.rs +++ b/src/binding_tests.rs @@ -306,4 +306,47 @@ mod bind_pk_value_tests { let err = bind_pk_value(&json!({"a": 1}), 1, None).unwrap_err(); assert!(err.contains("Unsupported PK type")); } + + // 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). Ported from the builtin driver's parity fix + // (TabularisDB/tabularis#618): a plain TEXT bind trips SQLSTATE 42883, + // "operator does not exist: numeric = text". These mirror #618's own + // test cases for `build_pk_predicate`, adapted to `bind_pk_value`'s + // signature. + + #[test] + fn numeric_column_string_value_casts_to_numeric() { + let bound = bind_pk_value(&json!("1500.00"), 2, Some("numeric")).unwrap(); + assert_eq!(bound.sql, "CAST($2 AS numeric)"); + let (_, pg_type) = bound.param.unwrap(); + assert_eq!(pg_type, tokio_postgres::types::Type::NUMERIC); + } + + #[test] + fn double_precision_column_string_value_casts_to_double() { + let bound = bind_pk_value(&json!("1.5"), 1, Some("double precision")).unwrap(); + assert_eq!(bound.sql, "CAST($1 AS double precision)"); + let (_, pg_type) = bound.param.unwrap(); + assert_eq!(pg_type, tokio_postgres::types::Type::FLOAT8); + } + + #[test] + fn numeric_column_unparsable_string_is_rejected() { + assert!(bind_pk_value(&json!("abc"), 1, Some("numeric")).is_err()); + } + + #[test] + fn timestamp_column_string_value_casts_through_text() { + let bound = bind_pk_value( + &json!("2024-05-01 10:30:00"), + 3, + Some("timestamp without time zone"), + ) + .unwrap(); + assert_eq!(bound.sql, "CAST($3 AS timestamp)"); + let (_, pg_type) = bound.param.unwrap(); + assert_eq!(pg_type, tokio_postgres::types::Type::TEXT); + } }