Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions internal/dbtest/issue1394_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package dbtest_test

import (
"context"
"testing"

"github.com/stretchr/testify/require"

"github.com/uptrace/bun"
)

// TestIssue1394_DroppedFieldValueOnSliceInsert reproduces issue #1394:
// when inserting a slice of structs, getFields() only inspected the first
// element to decide which fields marshal to DEFAULT (and should therefore
// go to the RETURNING clause instead of the VALUES clause). If a later
// element carried a real value for such a field, it was silently dropped
// from the INSERT and never reached the database.
//
// This test uses only SQLite so it runs without external services.
func TestIssue1394_DroppedFieldValueOnSliceInsert(t *testing.T) {
ctx := context.Background()

type Issue1394Model struct {
bun.BaseModel `bun:"table:issue_1394_models,alias:im"`

ID int64 `bun:",pk,autoincrement"`
Name string `bun:",notnull"`
Email string `bun:",notnull,default:'none'"`
}

db := sqlite(t)

mustResetModel(t, ctx, db, (*Issue1394Model)(nil))

// First element has a zero Email; second element has a real Email.
// Before the fix, getFields() inspected only the first element, saw
// Email is NotNull + marshalsToDefault (zero value + SQLDefault), and
// moved Email to RETURNING — so Email was dropped from the INSERT
// column list entirely. The second element's "alice@example.com" was
// silently replaced by the column's DEFAULT ('none').
models := []Issue1394Model{
{Name: "zero-email"},
{Name: "with-email", Email: "alice@example.com"},
}

_, err := db.NewInsert().Model(&models).Exec(ctx)
require.NoError(t, err)

var got []Issue1394Model
err = db.NewSelect().Model(&got).OrderExpr("im.id ASC").Scan(ctx)
require.NoError(t, err)

require.Len(t, got, 2)
require.Equal(t, "zero-email", got[0].Name)
require.Equal(t, "none", got[0].Email,
"first row had no email, so the column DEFAULT ('none') applies")
require.Equal(t, "with-email", got[1].Name)
require.Equal(t, "alice@example.com", got[1].Email,
"second row's Email must be persisted, not silently dropped (issue #1394)")
}
23 changes: 21 additions & 2 deletions query_insert.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ func (q *InsertQuery) getFields() ([]*schema.Field, error) {
}

var strct reflect.Value
var slice reflect.Value

switch model := q.tableModel.(type) {
case *structTableModel:
Expand All @@ -433,7 +434,7 @@ func (q *InsertQuery) getFields() ([]*schema.Field, error) {
if model.sliceLen == 0 {
return nil, fmt.Errorf("bun: Insert(empty %T)", model.slice.Type())
}
strct = indirect(model.slice.Index(0))
slice = model.slice
default:
return nil, errNilModel
}
Expand All @@ -445,7 +446,7 @@ func (q *InsertQuery) getFields() ([]*schema.Field, error) {
q.addReturningField(f)
continue
}
if f.NotNull && q.marshalsToDefault(f, strct) {
if f.NotNull && q.marshalsToDefaultForInsert(f, strct, slice) {
q.addReturningField(f)
continue
}
Expand All @@ -455,6 +456,24 @@ func (q *InsertQuery) getFields() ([]*schema.Field, error) {
return fields, nil
}

// marshalsToDefaultForInsert reports whether f marshals to DEFAULT/NULL for
// every row being inserted. For a single-struct model it checks the one
// struct; for a slice model it checks every element, so that a field is only
// moved to RETURNING when no element carries a real value. Otherwise a later
// element's value would be silently dropped from the INSERT (issue #1394).
func (q InsertQuery) marshalsToDefaultForInsert(f *schema.Field, strct, slice reflect.Value) bool {
if !slice.IsValid() {
return q.marshalsToDefault(f, strct)
}
n := slice.Len()
for i := 0; i < n; i++ {
if !q.marshalsToDefault(f, indirect(slice.Index(i))) {
return false
}
}
return true
}

// marshalsToDefault checks if the value will be marshaled as DEFAULT or NULL (if DEFAULT placeholder is not supported)
// when appending it to the VALUES clause in place of the given field.
func (q InsertQuery) marshalsToDefault(f *schema.Field, v reflect.Value) bool {
Expand Down
Loading