fix(postgres): show real values without float noise and keep row matching

[verbaux]

Refs #906

What

PostgreSQL real (float4) values reached the grid widened to f64, so 89.9 showed as 89.9000015258789.

• Display: FLOAT4 is serialized from the shortest f32 decimal (89.9). For the two f32 values whose shortest decimal parses to an exact f32 midpoint (±7.038531e-26), the exact widened value is used instead, so the JSON number always converts back to the stored f32. FLOAT8, NULL and NaN/Infinity (still null) are unchanged. Arrays, domains and composites go through the same scalar path.
• Row matching: after this change the grid sends 89.9 back, and real_col = 89.9::double precision matches nothing. When the column type is REAL/FLOAT4, the WHERE value is now cast to real (CAST(CAST($n AS double precision) AS real)). This covers UPDATE, DELETE and BLOB lookup, including composite keys and tables without a primary key.
• FK navigation: the filter for a real FK column uses CAST(<value> AS real) for the same reason.
• If the column type lookup fails, a warning is logged. Without the type, the cast is skipped and the row may not match.

The standalone PostgreSQL plugin gets the same fix on this branch.

MySQL FLOAT was separately confirmed to have the same display problem through a different code path; it is outside this PR.

Verification

• Regression tests cover finite extremes and both midpoint values, verifying that JSON round trips preserve the original f32.
• cargo test --lib drivers::postgres: 290 passed, 1 ignored.
• Parity test parity_real_primary_keys_round_trip_through_json against the plugin built from the branch above, PostgreSQL 18: single and composite real keys, including the midpoint values. Full parity run: 84 passed.
• pnpm exec vitest run tests/utils/foreignKeys.test.ts: 22 passed.

[kilo-code-bot]

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
src-tauri/src/drivers/postgres/mod.rs	638	Ok(None) (no column type found) logs at warn level. When get_column_data_type returns Ok(None) (column exists but type couldn't be determined), it logs at warn level. This is expected for some schemas (e.g., views, temporary tables) and could be debug instead. Keep warn for the Err case (actual failures).

Files Reviewed (8 files)

• src-tauri/src/drivers/postgres/binding.rs - No new issues (previous WARNING fixed)
• src-tauri/src/drivers/postgres/extract/simple.rs - No issues
• src-tauri/src/drivers/postgres/tests.rs - No issues
• src-tauri/tests/postgres_integration/parity_crud_extra.rs - No issues
• src/utils/cellValueFilter.ts - No issues
• src/utils/foreignKeys.ts - No new issues (previous SUGGESTION fixed)
• src/utils/identifiers.ts - No issues
• tests/utils/cellValueFilter.test.ts - No issues

Fix these issues in Kilo Cloud

Previous Review Summaries (2 snapshots, latest commit 68c4948)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 68c4948)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	2

Issue Details (click to expand)

WARNING

File	Line	Issue
src-tauri/src/drivers/postgres/binding.rs	96-105	String arm missing outer CAST(... AS real) for real/float4 columns (keyless tables). The fix for preserving float4 precision was applied to the Number arm (lines 64-72) but not to the String arm. When a keyless table has a real/float4 column and the PK value arrives as a JSON string (from the UI), it goes through bind_pg_numeric_string which returns CAST($N AS double precision). This needs to be wrapped as CAST(CAST($N AS double precision) AS real) so the comparison happens at the column's precision.

SUGGESTION

File	Line	Issue
src/utils/foreignKeys.ts	99	Uses shouldQuoteIdentifiers(driver) as PostgreSQL dialect proxy. shouldQuoteIdentifiers(driver) returns true for any dialect that uses double-quote identifiers (PostgreSQL, SQLite, etc.). For precision, prefer checking driver.capabilities?.sql_dialect === "postgres" (or driver.sql_dialect === "postgres" for string drivers) to specifically target PostgreSQL.
src-tauri/src/drivers/postgres/mod.rs	638	Ok(None) (no column type found) logs at warn level. When get_column_data_type returns Ok(None) (column exists but type couldn't be determined), it logs at warn level. This is expected for some schemas (e.g., views, temporary tables) and could be debug instead. Keep warn for the Err case (actual failures).

Files Reviewed (9 files)

• src-tauri/src/drivers/postgres/binding.rs - 1 WARNING
• src-tauri/src/drivers/postgres/extract/simple.rs - No issues
• src-tauri/src/drivers/postgres/extract/simple_tests.rs - No issues
• src-tauri/src/drivers/postgres/mod.rs - 1 SUGGESTION
• src-tauri/src/drivers/postgres/tests.rs - No issues
• src-tauri/tests/postgres_integration/parity_crud_extra.rs - No issues
• src-tauri/tests/postgres_integration/golden/execute_query_all_types.json - No issues
• src/utils/foreignKeys.ts - 1 SUGGESTION
• tests/utils/foreignKeys.test.ts - No issues

Fix these issues in Kilo Cloud

Previous review (commit 18311fd)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	2

Issue Details (click to expand)

WARNING

File	Line	Issue
src-tauri/src/drivers/postgres/binding.rs	97-104	Fix for real column precision missing in Value::String arm (keyless tables). The bind_pg_numeric_string path returns CAST($N AS double precision) without the outer CAST(... AS real), so row matching fails for keyless tables with real columns when PK values arrive as JSON strings.

SUGGESTION

File	Line	Issue
src/utils/foreignKeys.ts	99-101	Uses shouldQuoteIdentifiers(driver) as PostgreSQL dialect proxy. Could match other double-quote dialects (e.g., SQLite). Prefer driver.capabilities?.sql_dialect === "postgres" for precision.
src-tauri/src/drivers/postgres/mod.rs	638-643	Ok(None) (no column type found) logs at warn level; could be debug since it's expected for some schemas. Keep warn for Err (actual failures).

Files Reviewed (8 files)

• src-tauri/src/drivers/postgres/binding.rs - 1 WARNING
• src-tauri/src/drivers/postgres/extract/simple.rs - No issues
• src-tauri/src/drivers/postgres/extract/simple_tests.rs - No issues
• src-tauri/src/drivers/postgres/mod.rs - 1 SUGGESTION
• src-tauri/src/drivers/postgres/tests.rs - No issues
• src-tauri/tests/postgres_integration/parity_crud_extra.rs - No issues
• src/utils/foreignKeys.ts - 1 SUGGESTION
• tests/utils/foreignKeys.test.ts - No issues

Fix these issues in Kilo Cloud

---

_{Reviewed by free · Input: 203.7K · Output: 6.1K · Cached: 349.9K}

[kilo-code-bot]

SUGGESTION: Uses shouldQuoteIdentifiers(driver) as PostgreSQL dialect proxy

shouldQuoteIdentifiers(driver) returns true for any dialect that uses double-quote identifiers (PostgreSQL, SQLite, etc.). For precision, prefer checking driver.capabilities?.sql_dialect === "postgres" (or driver.sql_dialect === "postgres" for string drivers) to specifically target PostgreSQL.

Suggested change

	const isPostgresReal = shouldQuoteIdentifiers(driver) &&
	const isPostgresReal = driver.capabilities?.sql_dialect === "postgres" &&
	/^(real|float4)$/i.test(sourceColumnType?.trim() ?? "");

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[kilo-code-bot]

SUGGESTION: Ok(None) (no column type found) logs at warn level

When get_column_data_type returns Ok(None) (column exists but type couldn't be determined), it logs at warn level. This is expected for some schemas (e.g., views, temporary tables) and could be debug instead. Keep warn for the Err case (actual failures).

Suggested change

	Ok(None) => log::warn!(
	Ok(None) => log::debug!(

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[aesslinger]

@verbaux — really appreciate you tackling this across both repos. The float noise issue is a real user-facing pain point and this is a thorough, well-researched fix — the midpoint handling in the float4 extraction is a particularly nice piece of work, and the FK navigation CAST in foreignKeys.ts (including the MySQL non-cast guard) is well thought out.

We ran the same two independent review passes as on the plugin PR (#144) — an 11-dimension ensemble pre-handoff review and a focused code-review — and aggregated the results. Here's the summary:

Blocking (must address before merge)

1. Value::String PK path doesn't cast to real

The new Value::Number arm in build_pk_predicate wraps the bind in CAST(... AS real) for REAL/FLOAT4 columns, but the Value::String arm (used for keyless tables where all columns serialize as strings) routes through bind_pg_numeric_string which casts to double precision — no CAST(... AS real) wrapper. A keyless table with a REAL column where the row-identity value arrives as a JSON string would fail to match (double precision 89.9 ≠ 89.9000015258789 widened real). The number path was fixed; the string path was not.

Note: This same gap exists in the companion plugin PR (#144) — both PRs need the fix on their respective Value::String arms.

Nice-to-haves (low severity, not blocking)

• exec_insert silent swallow (src-tauri/src/drivers/postgres/crud.rs or the plugin equivalent): exec_update and exec_delete were updated to unwrap_or_else(|error| { log::warn!(...) }) for get_column_types_map failures, but exec_insert still uses unwrap_or_default(). The insert path's column-type failure is silently swallowed with no log, making the root cause invisible — the exact diagnostic gap the other two handlers were fixed to close.
• Vacuous unit test (simple_tests.rs): float4_extremes_round_trip_without_losing_precision only asserts round-trip (which both the old and new code satisfy), not the short-decimal output that distinguishes the fix. The sibling float4_preserves_short_decimal_values test is non-vacuous and covers this correctly — the extremes test just gives false confidence at the unit level.
• FLOAT4 match arm is effectively dead code: information_schema.columns.data_type returns "real" (not "float4"), so after extract_base_type uppercases, the "FLOAT4" arm in the matches! never fires for standard PostgreSQL columns. Harmless but misleading — a reader may assume FLOAT4 is a real alias the database returns.

What's clean

The rest of the PR is solid — security (no injection via CAST, no data leakage in logs), concurrency (no resource cleanup issues), error handling (NaN/Infinity → Null is correct, logging improvement is additive), backward compatibility (documented intentional fix, golden file updated consistently), test structure (all tests properly attributed, module wiring correct), boundary cases (all f32 edge values verified correct — NaN, Infinity, -0.0, MAX, MIN, subnormals, both midpoint values), and validation (REAL/FLOAT4 case-insensitivity is correct, foreignKeys.ts regex handles whitespace and undefined types).

Sequencing

This PR (#950) must merge before the companion plugin PR (#144). Once this lands, the builtin produces short decimals and the parity suite is green. Then #144 can merge (the plugin matches the builtin). Both PRs should address the string PK gap (Finding #1) before merging.

[debba]

Thanks a lot for this! The midpoint guard in the FLOAT4 decoder (if decimal as f32 == v { decimal } else { f64::from(v) }) is a really careful detail, and I like that you fixed the predicate side too instead of only cleaning up the display. The parity test with single and composite real keys, midpoints included, gives me a lot of confidence in the UPDATE/DELETE path.

There's one regression I'd like fixed before merging:

Filter by cell value on a real column no longer matches. buildCellValueFilterClause in src/utils/cellValueFilter.ts (the "filter by this value" context menu) still builds "price" = 89.9 with no cast. Postgres resolves real = 89.9 as a float4/float8 comparison, so it's false. I checked on Postgres 16: SELECT 89.9::real = 89.9 returns f, while 89.9::real = 89.9000015258789 and 89.9::real = CAST(89.9 AS real) both return t. Before this PR the cell carried 89.9000015258789, so = matched; now = returns no rows and <> returns every row, including the one you clicked. Could you apply the same CAST(... AS real) there as in buildForeignKeyFilterClause? Ideally both go through one small shared helper, with a test in tests/utils/cellValueFilter.test.ts.

Two small optional things:

• In extract/simple.rs the FLOAT4 conversion could be a small named helper (something like float4_to_json_f64) instead of living inline in the match arm. It reads a bit better and fits .rules/rust.md.
• In foreignKeys.ts, shouldQuoteIdentifiers(driver) works as a Postgres dialect check, but the name says something else. A named check (or the shared helper above) would make the intent clearer.

[verbaux]

@debba @aesslinger thanks for the reviews. Pushed:

• Filter by cell value on real (42c7b2a): = and <> now compare as CAST(<value> AS real) on PostgreSQL real/float4 columns. FK navigation and the cell-value filter share one helper, formatSqlValueForColumnComparison, with tests in tests/utils/cellValueFilter.test.ts.
• Dialect check: added isPostgresDialect. shouldQuoteIdentifiers delegates to it unchanged.
• FLOAT4 helper (aff0dbd): the conversion now lives in float4_to_json_f64 in extract/simple.rs.
• String PK path (20a73c4): numeric and numeric-string identities for real columns both compare as CAST(... AS real). Covered by a unit test and a parity test with a two-column keyless update; that test fails without the fix.
• Extremes test: it guards the conversion against precision loss rather than reproducing the bug, which float4_preserves_short_decimal_values does. Without the midpoint guard it fails for ±7.038531e-26.
• FLOAT4 arm: kept for consistency with the existing alias lists, e.g. bind_pg_numeric_string.
• exec_insert: that is the plugin's handler; insert builds no row predicate and the swallow predates this PR, so I left it out of scope.
• Sequencing: agreed, fix(postgres): show real values without float noise and keep row matching #950 first, then fix: show real values without float noise and keep row matching tabularis-postgresql-plugin#144.

[debba]

Hey @verbaux, thanks for turning this around so quickly. Moving both the FK navigation and the "filter by this value" path onto the same formatSqlValueForColumnComparison helper is exactly what I had in mind, and the new cases in tests/utils/cellValueFilter.test.ts cover the parts I cared about: = and <>, number and string values, double precision and MySQL left alone, and IS NULL untouched. I also appreciate that you closed the string identity gap in build_pk_predicate while you were at it, with the keyless parity test on top.

The two optional points are done as well: float4_to_json_f64 reads much better than the inline match arm, and isPostgresDialect makes the intent in foreignKeys.ts obvious.

I ran it on the branch: cargo test --lib drivers::postgres gives 290 passed, the cellValueFilter and foreignKeys suites pass (21 and 22), and CI is green including the Postgres integration job. Nothing left on my side, merging. Then the plugin PR can follow as you planned.