From 59fae2e7ae9e6a44a12244d4477b483d7721cffb Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Fri, 23 Oct 2026 22:52:04 -0600 Subject: [PATCH] fix(csvx): read human null words as absent, unrecognized values as false BoolishTimestamp defaulted unrecognized non-empty values to true: any word it does not understand (a misspelling, "disabled", "banana") behaved as on. Unrecognized values are now valid+false with the error retained so callers can apply their own policy, and "disabled"/"disable" are no longer true-like. Both bool types now read the human "not supplied" set through one shared isBoolNull sentinel: empty/whitespace-only, the exact PG dump sentinel \N (case-sensitive), and the case-insensitive words null, \null, none, unknown. Bool previously reached absent only by ParseBool failing; it now matches BoolishTimestamp explicitly. Free-text types must not reuse this set: most of these words are legitimate values there. --- database/csvx/bool.go | 7 ++++- database/csvx/boolish.go | 16 +++++++---- database/csvx/csvx_test.go | 40 +++++++++++++++++++++++++--- database/csvx/null.go | 22 ++++++++++++++++ database/csvx/null_test.go | 54 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 9 deletions(-) create mode 100644 database/csvx/null.go create mode 100644 database/csvx/null_test.go diff --git a/database/csvx/bool.go b/database/csvx/bool.go index 0f84937..3a95753 100644 --- a/database/csvx/bool.go +++ b/database/csvx/bool.go @@ -34,6 +34,11 @@ func (b *Bool) Scan(value any) error { b.Bool = v b.Valid = true case string: + if isBoolNull(v) { + b.Valid = false + b.Bool = false + return nil + } parsed, err := strconv.ParseBool(strings.TrimSpace(v)) if err != nil { b.Valid = false @@ -61,7 +66,7 @@ func (b Bool) MarshalCSV() ([]byte, error) { func (b *Bool) UnmarshalCSV(data []byte) error { s := strings.TrimSpace(string(data)) - if s == "" { + if isBoolNull(s) { b.Valid = false b.Bool = false return nil diff --git a/database/csvx/boolish.go b/database/csvx/boolish.go index d5951af..757e192 100644 --- a/database/csvx/boolish.go +++ b/database/csvx/boolish.go @@ -17,19 +17,23 @@ type BoolishTimestamp struct { Error error } -// UnmarshalCSV parses a human value. Unknown non-empty values are retained as -// errors so the collection mapper can apply its own conservative policy. +// UnmarshalCSV parses a human value. Recognized true-like values and dates +// set Bool; recognized false-like values leave it unset. Unrecognized +// non-empty values are conservatively false with the error retained so the +// collection mapper can apply its own policy. func (v *BoolishTimestamp) UnmarshalCSV(data []byte) error { *v = BoolishTimestamp{} raw := strings.TrimSpace(string(data)) - if raw == "" || strings.EqualFold(raw, "null") || raw == `\N` { + // Null sentinels (isBoolNull): absent means Valid=false. `\N` is the + // exact PG dump sentinel and case-sensitive; the word forms are not. + if isBoolNull(raw) { return nil } v.Valid = true switch strings.ToLower(raw) { case "false", "f", "n", "no", "0": return nil - case "true", "t", "y", "yes", "1", "disabled", "disable": + case "true", "t", "y", "yes", "1": v.Bool = true return nil } @@ -41,7 +45,9 @@ func (v *BoolishTimestamp) UnmarshalCSV(data []byte) error { return nil } } - v.Bool = true + // Unrecognized: valid but false, error recorded (Bool must not default + // to true for values this type does not understand). + v.Bool = false v.Error = fmt.Errorf("unrecognized boolish timestamp %q", raw) return nil } diff --git a/database/csvx/csvx_test.go b/database/csvx/csvx_test.go index ed45be5..481cb64 100644 --- a/database/csvx/csvx_test.go +++ b/database/csvx/csvx_test.go @@ -204,11 +204,45 @@ func TestBoolishTimestamp(t *testing.T) { } // False-like - if err := v.UnmarshalCSV([]byte("no")); err != nil { + for _, falseish := range []string{"no", "0", "FALSE"} { + if err := v.UnmarshalCSV([]byte(falseish)); err != nil { + t.Fatalf("UnmarshalCSV(%q): %v", falseish, err) + } + if !v.Valid || v.Bool || v.Error != nil { + t.Fatalf("%q: expected valid+false, got %+v", falseish, v) + } + } + + // An unrecognized word must not be treated as true: it is valid but + // false with an error recorded. The Bool-family null words are sentinels + // (covered by TestIsBoolNull), so use words outside that set. + for _, word := range []string{"random", "disabled"} { + if err := v.UnmarshalCSV([]byte(word)); err != nil { + t.Fatalf("UnmarshalCSV(%q): %v", word, err) + } + if !v.Valid || v.Bool || v.Error == nil { + t.Fatalf("%q: expected valid+false+error, got %+v", word, v) + } + } + + // Empty / null sentinels: `\N` exact; word forms any case. The full + // set is covered by TestIsBoolNull. + for _, sent := range []string{"", "null", `\N`} { + if err := v.UnmarshalCSV([]byte(sent)); err != nil { + t.Fatalf("UnmarshalCSV(%q): %v", sent, err) + } + if v.Valid || v.Bool || v.Error != nil { + t.Fatalf("%q: expected absent, got %+v", sent, v) + } + } + + // `\n` (lowercase) is NOT the PG sentinel: unrecognized, so valid + + // false + error. + if err := v.UnmarshalCSV([]byte(`\n`)); err != nil { t.Fatalf("UnmarshalCSV: %v", err) } - if !v.Valid || v.Bool { - t.Fatalf("expected valid+false, got %+v", v) + if !v.Valid || v.Bool || v.Error == nil { + t.Fatalf(`\n: expected valid+false+error, got %+v`, v) } // Empty diff --git a/database/csvx/null.go b/database/csvx/null.go new file mode 100644 index 0000000..327b255 --- /dev/null +++ b/database/csvx/null.go @@ -0,0 +1,22 @@ +package csvx + +import "strings" + +// isBoolNull reports whether a human-edited cell means "not supplied": +// empty/whitespace-only, the exact PG dump sentinel `\N` (case-sensitive), +// or a case-insensitive match of null, \null, none, or unknown. +// +// Bool and BoolishTimestamp read such cells as absent. Free-text types +// must not reuse this set wholesale: most of these words are legitimate +// text values in non-boolean contexts. +func isBoolNull(raw string) bool { + s := strings.TrimSpace(raw) + if s == "" || s == `\N` { + return true + } + switch strings.ToLower(s) { + case "null", `\null`, "none", "unknown": + return true + } + return false +} diff --git a/database/csvx/null_test.go b/database/csvx/null_test.go new file mode 100644 index 0000000..baa4c6c --- /dev/null +++ b/database/csvx/null_test.go @@ -0,0 +1,54 @@ +package csvx + +import "testing" + +func TestIsBoolNull(t *testing.T) { + for _, null := range []string{"", " ", "null", "NULL", "Null", " null ", `\null`, `\NULL`, `\N`, "none", "None", "unknown", "UNKNOWN"} { + if !isBoolNull(null) { + t.Errorf("isBoolNull(%q) = false, want true", null) + } + } + // Everything else is a value, not a null. + for _, value := range []string{`\n`, `N`, `N\`, "none of the above", "nullable", "no", "off", "na", "disable", "disabled", "NULLITY"} { + if isBoolNull(value) { + t.Errorf("isBoolNull(%q) = true, want false", value) + } + } +} + +func TestBoolCSVNullSentinels(t *testing.T) { + // Bool must read the same null sentinels as BoolishTimestamp (isBoolNull) + // explicitly rather than relying on ParseBool failing: the PG dump + // sentinel and a representative word. + for _, null := range []string{`\N`, "none"} { + var b Bool + if err := b.UnmarshalCSV([]byte(null)); err != nil { + t.Fatalf("UnmarshalCSV(%q): %v", null, err) + } + if b.Valid || b.Bool { + t.Errorf("UnmarshalCSV(%q) = %+v, want absent", null, b) + } + var b2 Bool + if err := b2.Scan(null); err != nil { + t.Fatalf("Scan(%q): %v", null, err) + } + if b2.Valid || b2.Bool { + t.Errorf("Scan(%q) = %+v, want absent", null, b2) + } + } + // `\n` (lowercase) is not the sentinel: it parses as unrecognized. + var b Bool + if err := b.Scan(`\n`); err != nil { + t.Fatalf("Scan: %v", err) + } + if b.Valid || b.Bool { + t.Errorf(`Scan(\n) = %+v, want absent`, b) + } + // Real values still parse; a non-sentinel word is unrecognized. + if err := b.UnmarshalCSV([]byte("true")); err != nil { + t.Fatalf("UnmarshalCSV(true): %v", err) + } + if !b.Valid || !b.Bool { + t.Errorf("UnmarshalCSV(true) = %+v", b) + } +}