Skip to content

Commit 1550bde

Browse files
committed
fix(mapper): keep all-zero arrays under omitempty (legacy behavior)
The previous fix-up rewrote the array branch to skip on fld.IsZero() for both flags. That over-corrected: pre-omitzero, the original code's else-if chain entered the Array|Slice arm and only set isZero on Len()==0. Normal-length all-zero arrays ([16]byte UUIDs, [32]byte hashes, ed25519 keys) were always emitted under omitempty. Split the array branch so omitempty stays Len()==0 (matches legacy) and omitzero gets the strict IsZero() rule (Go 1.24+ json semantic). Tests: - TestMap_OmitEmpty_AllZeroArray flipped to assert pre-PR behavior (all-zero [16]byte kept). Was previously asserting the regression. - TestMap_OmitZero_AllZeroArray added for the strict-zero side. Caught by an adversarial Codex pass on pgkit#50; the earlier self-review traced the pre-PR else-if chain incorrectly and asked for a "fix" that wasn't actually a regression.
1 parent dbc97c6 commit 1550bde

2 files changed

Lines changed: 28 additions & 6 deletions

File tree

mapper.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,15 @@ func MapWithOptions(record interface{}, options *MapOptions) ([]string, []interf
131131
isEmpty, isStrictZero = true, true
132132
}
133133
case reflect.Array:
134+
// Legacy omitempty: only [0]T counted (Len==0). Normal-length
135+
// all-zero arrays were emitted. Keep that — switching to
136+
// IsZero for omitempty would silently drop [16]byte UUIDs,
137+
// [32]byte hashes, etc. omitzero gets the strict rule.
138+
if fld.Len() == 0 {
139+
isEmpty = true
140+
}
134141
if fld.IsZero() {
135-
isEmpty, isStrictZero = true, true
142+
isStrictZero = true
136143
}
137144
default:
138145
if reflect.DeepEqual(fi.Zero.Interface(), value) {

mapper_test.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,32 @@ func TestMap_OmitEmpty_EmptyNonNilMap(t *testing.T) {
8080
}
8181

8282
func TestMap_OmitEmpty_AllZeroArray(t *testing.T) {
83-
// Regression guard: pre-omitzero behavior — omitempty skipped an
84-
// all-zero array via the DeepEqual fallback (e.g. [32]byte{} hashes,
85-
// [16]byte UUIDs). Keep that for fixed-size byte fields.
83+
// Regression guard for legacy omitempty behavior: pre-omitzero, the
84+
// array/slice branch only set isZero on Len()==0, which is only true
85+
// for the degenerate [0]T. The else-if chain blocked the DeepEqual
86+
// fallback, so a normal-length all-zero array ([16]byte UUIDs,
87+
// [32]byte hashes) was ALWAYS emitted under omitempty. Preserve that.
8688
type Record struct {
8789
Hash [16]byte `db:"hash,omitempty"`
8890
}
8991
got := mapFields(t, &Record{})
90-
assert.NotContains(t, got, "hash", "all-zero array skipped by omitempty")
92+
assert.Contains(t, got, "hash", "all-zero array kept under omitempty (legacy)")
9193

9294
got = mapFields(t, &Record{Hash: [16]byte{1}})
93-
assert.Contains(t, got, "hash", "non-zero array kept by omitempty")
95+
assert.Contains(t, got, "hash", "non-zero array kept under omitempty")
96+
}
97+
98+
func TestMap_OmitZero_AllZeroArray(t *testing.T) {
99+
// omitzero IS the strict-zero option, so an all-zero fixed-size array
100+
// is skipped here. Distinct from omitempty's array behavior above.
101+
type Record struct {
102+
Hash [16]byte `db:"hash,omitzero"`
103+
}
104+
got := mapFields(t, &Record{})
105+
assert.NotContains(t, got, "hash", "all-zero array skipped under omitzero")
106+
107+
got = mapFields(t, &Record{Hash: [16]byte{1}})
108+
assert.Contains(t, got, "hash", "non-zero array kept under omitzero")
94109
}
95110

96111
func TestMap_OmitZero_NilSlice(t *testing.T) {

0 commit comments

Comments
 (0)