Skip to content

feat(builder): union columns + DEFAULT for mixed-shape InsertRecords#54

Merged
klaidliadon merged 1 commit into
masterfrom
fix-insertrecords-mixed-shape
Jun 10, 2026
Merged

feat(builder): union columns + DEFAULT for mixed-shape InsertRecords#54
klaidliadon merged 1 commit into
masterfrom
fix-insertrecords-mixed-shape

Conversation

@klaidliadon

Copy link
Copy Markdown
Contributor

#50 rejected any batch whose rows produced different Map column sets — ,omitzero slice / ,omitempty map records that mix nil and non-nil empty values hit this constantly, forcing callers to split into per-row InsertRecord calls. Closes #53.

records := []*Order{
    {Name: "first"},                              // cols = [name]
    {Name: "second", Tags: []string{"a", "b"}},   // cols = [name, tags]
    {Name: "third",  Note: &note},                // cols = [name, note]
}
DB.SQL.InsertRecords(records, "orders")
// → INSERT INTO orders (name,note,tags) VALUES
//   ($1,DEFAULT,DEFAULT),($2,DEFAULT,$3),($4,$5,DEFAULT)

PostgreSQL accepts DEFAULT in any VALUES position. The fix: union the cols across rows, emit sq.Expr("DEFAULT") for any slot a row skipped.


Design

Drafted via three Codex review rounds. Plan at tmp/insertrecords-mixed-shape/2026-06-02-plan.md (branch-only).

Key inflection points the reviews forced:

  1. v1 had per-row empty-record rejection — kept feat(mapper): support ,omitzero tag option #50's behavior of erroring when any individual row mapped to zero cols. Codex blocker: that's now arbitrary. If another row contributes the union, a zero-column row is representable as (DEFAULT, DEFAULT, ...). Only the whole-batch empty union should error.
  2. v1 padded by slice position — fragile. v2 pads by column name via map[string]any per row.
  3. v2 hinted at SQL.InsertDefaults in the whole-batch-empty error — but that API is on the unmerged feat(builder): InsertDefaults for INSERT ... DEFAULT VALUES #52 branch. v3 hints at sq.Expr (matches feat(mapper): support ,omitzero tag option #50's existing single-row hint) so InsertRecords: union columns + emit DEFAULT for missing, instead of rejecting mixed-shape batches #53 stays independent of feat(builder): InsertDefaults for INSERT ... DEFAULT VALUES #52. Whichever lands second updates its own hint.

Reuses sqlDefault = sq.Expr("DEFAULT") from mapper.go:26. slices.Sorted(maps.Keys(colSet)) matches Map's lexical column order at mapper.go:161, so generated SQL lines up with what callers see from Map(record) directly.

Behavior table

Batch Before (#50) After (#53)
All rows same shape, non-empty works works (unchanged)
Mixed shapes, all rows non-empty rejected (drift) unions cols + DEFAULT fills
Some rows empty, some non-empty rejected (drift + empty) empty rows become all-DEFAULT
All rows empty rejected (per-row empty) rejected (whole-batch empty)
Empty slice of records rejected (existing) rejected (existing)

Test plan

  • make db-reset test-all against PostgreSQL 18.3 — all 6 packages green.
  • 7 unit tests: uniform shape (with args assertion), mixed shape with DEFAULT slots in the right places, ,omitzero mixed slices, ,omitempty mixed maps, empty row mixed with non-empty, all-rows-empty rejection, heterogeneous map[string]any records.
  • Integration TestInsertRecordsMixedShapeRoundTrip: exec a three-row heterogeneous batch against a new dedicated mixed_shape table (nullable + defaulted columns), verify each row landed with the right mix of caller values / DB defaults / NULLs.
  • go vet ./... clean.

Notes

  • ,omitempty semantics preserved: rows where ,omitempty skipped a column (e.g. []string{} on a slice field) get DEFAULT in the union slot, not the empty literal. The user tagged the column for "use DB default when missing" — that contract still holds, just now inside a batch.
  • Hint coordination: if #52 lands first, this PR's whole-batch-empty error could be updated to point at SQL.InsertDefaults instead of sq.Expr. Independent edit, no merge ordering required.

@marino39 marino39 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@klaidliadon klaidliadon force-pushed the fix-insertrecords-mixed-shape branch from 006a90f to 8fba979 Compare June 10, 2026 12:31
Closes #53. Follow-up to #50.

Replace the InsertRecords column-drift check with union-by-name: walk rows once to compute the column union and a per-row map of present columns, then emit Columns(allCols...) with each row padded to the union via DEFAULT where the row skipped a column.

The per-row empty-record rejection from #50 goes away because an empty row can be all-DEFAULT when another row contributes the union. Only the whole-batch empty case still errors, now pointing callers at SQL.InsertDefaults in a loop after rebasing over the dedicated DEFAULT VALUES builder.

Tests cover uniform and mixed shapes, omitzero slices, mixed maps, empty rows with a non-empty union, all-empty rejection, map records, InsertDefaults SQL construction, and PG roundtrips for both InsertDefaults and mixed-shape inserts.
@klaidliadon klaidliadon force-pushed the fix-insertrecords-mixed-shape branch from 8fba979 to bdaa64b Compare June 10, 2026 12:32
@klaidliadon klaidliadon merged commit bc2a081 into master Jun 10, 2026
1 check passed
@klaidliadon klaidliadon deleted the fix-insertrecords-mixed-shape branch June 10, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InsertRecords: union columns + emit DEFAULT for missing, instead of rejecting mixed-shape batches

2 participants