fix: filter NULL column defaults case-insensitively to match builtin (#122) - #125
Merged
Merged
Conversation
…122) row_to_table_column's default_value filter used a case-sensitive `d == "NULL"`, so a column declared `DEFAULT null` (lowercase — legal PostgreSQL, stored verbatim in information_schema.columns.column_default) surfaced a spurious `default_value: "null"` in the wire JSON, where the built-in driver omits the field. The builtin filters case-insensitively (`eq_ignore_ascii_case("null")`) in both get_columns and get_all_columns_batch. Affects get_columns, get_all_columns_batch, and get_schema_snapshot, which all share the row_to_table_column mapper. Fixed by matching the builtin exactly: the bare-null check is now case-insensitive, while the `NULL::<type>` cast-prefix check stays case-sensitive (the builtin keeps that one starts_with("NULL::"), not eq_ignore_ascii_case). Extracted the filter into a pure column_default_value(column_default, is_identity) helper split out of row_to_table_column for unit testing without a live tokio_postgres::Row, per the repo's extract-pure-logic pattern. TDD: added 5 unit tests for column_default_value covering the divergence (lowercase `null` — the bug), mixed-case `Null`, uppercase `NULL`, `NULL::` casts, real defaults, auto-increment (nextval + is_identity), and empty/None. Confirmed the lowercase-null test failed against the pre-fix code for the right reason (returned Some("null") instead of None) before applying the fix; all 5 pass after. Verified: cargo build --release; 361 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. Note: the 83-test parity suite's seed fixtures declare no `DEFAULT null` columns, so this divergence was never caught by it — only a source-level audit of the builtin's filter found it (#122).
aesslinger
added
bug
Version suggestionBased on this PR's title (
This is informational only — no tag or release is created automatically yet. |
The #122 fix extracted column_default_value with its own is_auto_increment computation, duplicating the one row_to_table_column already computes for the is_auto_increment field. The builtin computes is_auto once and reuses it for both the field and the filter; match that structure so the two can't drift. column_default_value now takes the pre-computed bool instead of is_identity, and the unit tests pass it through. Also adds a test guarding that a string *literal* 'null' (stored by PG as 'null'::text, quoted+cast) survives the filter — it doesn't match the bare-null check — which is the empirical boundary the live-probe confirmed. No behavior change: is_auto_increment was already computed identically in both places; this just dedupes it. 361 unit + 30 live-DB + 83/83 parity all pass.
aesslinger
added a commit
that referenced
this pull request
Sep 30, 2026
Ships the two PRs merged since 1.0.0-rc.5: the NULL column-default filter parity fix (#125, #122 — case-insensitive to match the builtin) and the cancel server-side statement notification handler (#127, #126 — dormant until tabularis#832 ships the host-side notification). Verified: cargo build --release; 366 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.
What
Fixes #122 — a byte-for-byte parity divergence in the
default_valuefilter shared byget_columns,get_all_columns_batch, andget_schema_snapshot.row_to_table_columnfiltered outNULLcolumn defaults case-sensitively (d == "NULL"), but the built-in driver filters case-insensitively (d.eq_ignore_ascii_case("null")) in both itsget_columnsandget_all_columns_batch. So a column declaredDEFAULT null(lowercase — legal PostgreSQL, stored verbatim ininformation_schema.columns.column_default) made this plugin emit a spuriousdefault_value: "null"in the wire JSON, where the builtin omits the field entirely.Fix
Match the builtin exactly:
nullcheck is now case-insensitive (eq_ignore_ascii_case("null")) — dropsNULL,null,Null, etc.NULL::<type>cast-prefix check stays case-sensitive (starts_with("NULL::")) — the builtin keeps that onestarts_with("NULL::"), noteq_ignore_ascii_case, so this matches.Extracted the filter into a pure
column_default_value(column_default, is_identity)helper split out ofrow_to_table_column, for unit testing without a livetokio_postgres::Row(per the repo's extract-pure-logic pattern,.rules/rust.md#4/#5).row_to_table_columnnow calls it.Why the parity suite didn't catch this
The 83-test cross-repo parity suite's seed fixtures (
tabularis/tests/fixtures/postgres_seed.sql) declare noDEFAULT nullcolumns (onlyDEFAULT 'neutral'andDEFAULT 0), so the divergence was never exercised — it was found by a source-level audit of the builtin's filter during the #123 hard verification, not by the suite. Filed separately as #122 per scope discipline rather than folded into #123.TDD
Per
CLAUDE.md's TDD-for-parity-bugs pattern: proved the divergence with a unit test before fixing. Added 5column_default_valuetests covering the divergence (lowercasenull— the bug), mixed-caseNull, uppercaseNULL,NULL::casts, real defaults (0,'neutral',now()), auto-increment (nextval(...)+is_identity=YES), and empty/None. Confirmedcolumn_default_value_drops_a_lowercase_null_default_to_match_builtinfailed against the pre-fix code for the right reason (Some("null")instead ofNone) before applying the fix; all 5 pass after.Verification
cargo build --releasePOSTGRES_PLUGIN_BINagainsttabularis'src-tauri/tests/postgres_integration parity*): 83/83 pass — no existing behavior regressed.postgres:16instance.cargo clippy --all-targets -- -D warnings;cargo fmt --all --check;npx markdownlint CHANGELOG.md— all clean.Closes #122.