diff --git a/CHANGELOG.md b/CHANGELOG.md index 7aa5180..3a43b5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ ## [Unreleased] +### Fixed + +- `get_columns`/`get_all_columns_batch`/`get_schema_snapshot` filtered out + `NULL` column defaults case-sensitively (`d == "NULL"`), so a column + declared `DEFAULT null` (lowercase — legal PostgreSQL) surfaced a + spurious `default_value: "null"` where the built-in driver omits it. + Now filters case-insensitively (`eq_ignore_ascii_case("null")`), + matching the builtin exactly; the `NULL::` cast prefix check + stays case-sensitive as in the builtin (#122). + ## [1.0.0-rc.5] - 2026-09-29 ### Added diff --git a/src/handlers/metadata.rs b/src/handlers/metadata.rs index 0b3d344..1b038b1 100644 --- a/src/handlers/metadata.rs +++ b/src/handlers/metadata.rs @@ -284,13 +284,7 @@ fn row_to_table_column(r: &tokio_postgres::Row) -> Value { let is_nullable = is_nullable_str == "YES"; - let default_value = column_default.as_deref().and_then(|d| { - if is_auto_increment || d.is_empty() || d == "NULL" || d.starts_with("NULL::") { - None - } else { - Some(d.to_string()) - } - }); + let default_value = column_default_value(column_default.as_deref(), is_auto_increment); let mut col = json!({ "name": name, @@ -319,6 +313,35 @@ fn row_to_table_column(r: &tokio_postgres::Row) -> Value { col } +/// Compute a column's `default_value` wire field from its raw +/// `information_schema.columns.column_default` and the already-computed +/// `is_auto_increment`, mirroring the built-in driver's filter exactly. +/// Returns `None` (omit the field) for auto-increment columns, empty +/// defaults, and `NULL` defaults of any case (`NULL`, `null`, `Null`, ...), +/// and for `NULL::` casts; everything else is surfaced verbatim. +/// Split out of `row_to_table_column` for unit testing without a live +/// `tokio_postgres::Row` (#122). `is_auto_increment` is computed once in +/// the caller (matching the builtin's single computation) and passed in +/// so the field and the filter can't drift apart. +fn column_default_value(column_default: Option<&str>, is_auto_increment: bool) -> Option { + let d = column_default?; + // Match the builtin's filter exactly: the bare-`null` check is + // case-insensitive (PostgreSQL accepts `DEFAULT null` in any case and + // normalizes it to NULL in information_schema.columns.column_default, + // so `null`/`Null`/`NULL` all mean "no default"), while the + // `NULL::` cast prefix stays case-sensitive — the builtin keeps + // that one `starts_with("NULL::")`, not `eq_ignore_ascii_case`. #122. + if is_auto_increment + || d.is_empty() + || d.eq_ignore_ascii_case("null") + || d.starts_with("NULL::") + { + None + } else { + Some(d.to_string()) + } +} + pub async fn get_foreign_keys(id: Value, params: &Value) -> Value { let conn_params = ConnectionParams::from_value(inner_params(params)); let table = params.get("table").and_then(Value::as_str).unwrap_or(""); diff --git a/src/handlers/metadata_tests.rs b/src/handlers/metadata_tests.rs index e7c6291..5473b77 100644 --- a/src/handlers/metadata_tests.rs +++ b/src/handlers/metadata_tests.rs @@ -7,7 +7,7 @@ //! (`.rules/rust.md` #4/#5) — loaded via //! `#[cfg(test)] #[path = "metadata_tests.rs"] mod metadata_tests;`. -use super::{build_table_ddl, routine_query_for_version}; +use super::{build_table_ddl, column_default_value, routine_query_for_version}; use serde_json::{json, Map, Value}; #[test] @@ -248,3 +248,102 @@ fn build_schema_snapshot_drops_table_comments_to_match_builtin_table_schema() { "snapshot entry must be exactly the builtin's TableSchema shape (name/columns/foreign_keys), dropping comment" ); } + +#[test] +fn column_default_value_drops_a_lowercase_null_default_to_match_builtin() { + // #122: the builtin filters NULL defaults case-insensitively + // (eq_ignore_ascii_case("null")), so `DEFAULT null` (lowercase, which + // PostgreSQL accepts) must produce no default_value — same as + // `DEFAULT NULL`. The plugin's filter was case-sensitive (`== "NULL"`), + // so it emitted a spurious `default_value: "null"` here. This is the + // divergence the test proves before the fix. + // + // The second arg is the caller's already-computed `is_auto_increment` + // (false here — these aren't sequence/identity columns). + assert_eq!( + column_default_value(Some("null"), false), + None, + "lowercase `null` default must be dropped, matching the builtin's case-insensitive filter" + ); + assert_eq!( + column_default_value(Some("NULL"), false), + None, + "uppercase `NULL` default must be dropped (this case already worked)" + ); + assert_eq!( + column_default_value(Some("Null"), false), + None, + "mixed-case `Null` default must be dropped, matching the builtin's case-insensitive filter" + ); +} + +#[test] +fn column_default_value_drops_null_cast_prefixes() { + // `NULL::` casts are PostgreSQL's representation of a nullable + // column with no real default; the builtin drops these too. The + // `NULL::` prefix check stays case-sensitive in the builtin (only the + // bare-null check is eq_ignore_ascii_case), so match that exactly. + assert_eq!(column_default_value(Some("NULL::text"), false), None); + assert_eq!(column_default_value(Some("NULL::integer"), false), None); +} + +#[test] +fn column_default_value_surfaces_real_defaults_unchanged() { + // Non-NULL defaults pass through verbatim — the value the host shows + // in the column's default cell. (A string literal `'null'` is stored + // by PG as `'null'::text`, which doesn't match the bare-null check and + // so correctly survives as a real default.) + assert_eq!( + column_default_value(Some("0"), false), + Some("0".to_string()) + ); + assert_eq!( + column_default_value(Some("'neutral'"), false), + Some("'neutral'".to_string()) + ); + assert_eq!( + column_default_value(Some("now()"), false), + Some("now()".to_string()) + ); + assert_eq!( + column_default_value(Some("'null'::text"), false), + Some("'null'::text".to_string()), + "a string *literal* 'null' is a real default (PG stores it quoted+cast) and must survive, not be dropped as the bare keyword" + ); +} + +#[test] +fn column_default_value_drops_auto_increment_defaults() { + // SERIAL/IDENTITY columns carry a `nextval(...)` default or an + // is_identity of YES; the host surfaces those as is_auto_increment, + // not as default_value, so the raw default must not leak through. + // The caller computes is_auto_increment once (matching the builtin's + // single computation) and passes it in, so the helper just honors it. + assert_eq!( + column_default_value(Some("nextval('users_id_seq'::regclass)"), true), + None, + "a nextval default the caller flagged as auto-increment must be dropped" + ); + assert_eq!( + column_default_value(Some("42"), true), + None, + "an is_identity=YES column (caller sets is_auto_increment=true) drops the default regardless of its value" + ); + // Negative control: the same `42` default on a non-auto column survives. + assert_eq!( + column_default_value(Some("42"), false), + Some("42".to_string()) + ); +} + +#[test] +fn column_default_value_drops_empty_defaults() { + // An empty string is not a meaningful default — drop it rather than + // surfacing an empty default_value field. + assert_eq!(column_default_value(Some(""), false), None); + assert_eq!( + column_default_value(None, false), + None, + "no default at all -> None" + ); +}