UpdateQuery: marshal false as BOOLEAN if nullzero tag not set (#937)

* test: capture bug * test: add test cases for nullzero + default * fix: marshal bool zero values as sql boolean * refactor: move complex conditional to a method The `f.SQLDefault == ""` part seems redundant before addReturningField, as RETURNING makes sense for a field that was given its default value. * test: add test case for empty string + nullzero
uptrace · Jan 10, 2024 · f6999e1 · f6999e1
1 parent d369a41
commit f6999e1
Show file tree

Hide file tree

Showing 31 changed files with 77 additions and 30 deletions.
diff --git a/internal/dbtest/query_test.go b/internal/dbtest/query_test.go
@@ -331,20 +331,27 @@ func TestQuery(t *testing.T) {
 				Where("model.id = _data.id")
 		},
 		func(db *bun.DB) schema.QueryAppender {
+			// "nullzero" marshals zero values as DEFAULT or NULL (if DEFAULT placeholder is not supported)
+			// DB drivers which support DEFAULT placeholder resolve it to NULL for columns that do not have a DEFAULT value.
 			type Model struct {
-				Int  int64     `bun:",nullzero"`
-				Uint uint64    `bun:",nullzero"`
-				Str  string    `bun:",nullzero"`
-				Time time.Time `bun:",nullzero"`
+				Int      int64     `bun:",nullzero"`
+				Uint     uint64    `bun:",nullzero"`
+				Str      string    `bun:",nullzero"`
+				Time     time.Time `bun:",nullzero"`
+				Bool     bool      `bun:",nullzero"`
+				EmptyStr string    `bun:",nullzero"` // same as Str
 			}
 			return db.NewInsert().Model(new(Model))
 		},
 		func(db *bun.DB) schema.QueryAppender {
+			// "nullzero,default" is equivalent to "default", marshalling zero values to DEFAULT
 			type Model struct {
-				Int  int64     `bun:",nullzero,default:42"`
-				Uint uint64    `bun:",nullzero,default:42"`
-				Str  string    `bun:",nullzero,default:'hello'"`
-				Time time.Time `bun:",nullzero,default:now()"`
+				Int      int64     `bun:",nullzero,default:42"`
+				Uint     uint64    `bun:",nullzero,default:42"`
+				Str      string    `bun:",nullzero,default:'hello'"`
+				Time     time.Time `bun:",nullzero,default:now()"`
+				Bool     bool      `bun:",nullzero,default:true"`
+				EmptyStr string    `bun:",nullzero,default:''"`
 			}
 			return db.NewInsert().Model(new(Model))
 		},
@@ -1016,6 +1023,28 @@ func TestQuery(t *testing.T) {
 			_ = q.String()
 			return q
 		},
+		func(db *bun.DB) schema.QueryAppender {
+			return db.NewUpdate().Model(&struct {
+				bun.BaseModel `bun:"table:accounts"`
+				ID            int  `bun:"id,pk,autoincrement"`
+				IsActive      bool `bun:"is_active,notnull,default:true"`
+			}{
+				ID:       1,
+				IsActive: false,
+			}).Column("is_active").WherePK()
+		},
+		func(db *bun.DB) schema.QueryAppender {
+			// "default" marshals zero values as DEFAULT or the specified default value
+			type Model struct {
+				Int      int64     `bun:",default:42"`
+				Uint     uint64    `bun:",default:42"`
+				Str      string    `bun:",default:'hello'"`
+				Time     time.Time `bun:",default:now()"`
+				Bool     bool      `bun:",default:true"`
+				EmptyStr string    `bun:",default:''"`
+			}
+			return db.NewInsert().Model(new(Model))
+		},
 	}
 
 	timeRE := regexp.MustCompile(`'2\d{3}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}(\.\d+)?(\+\d{2}:\d{2})?'`)

diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-165 b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-165
@@ -0,0 +1 @@
+UPDATE `accounts` SET `is_active` = FALSE WHERE (`accounts`.`id` = 1)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-166 b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-166
@@ -0,0 +1 @@
+INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING `int`, `uint`, `str`, `time`, `bool`, `empty_str`
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-51 b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-51
@@ -1 +1 @@
-INSERT INTO `models` (`int`, `uint`, `str`, `time`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING `int`, `uint`, `str`, `time`
+INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING `int`, `uint`, `str`, `time`, `bool`, `empty_str`
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-52 b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-52
@@ -1 +1 @@
-INSERT INTO `models` (`int`, `uint`, `str`, `time`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING `int`, `uint`, `str`, `time`
+INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING `int`, `uint`, `str`, `time`, `bool`, `empty_str`
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-165 b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-165
@@ -0,0 +1 @@
+UPDATE "accounts" SET "is_active" = 0 WHERE ("id" = 1)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-166 b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-166
@@ -0,0 +1 @@
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-51 b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-51
@@ -1 +1 @@
-INSERT INTO "models" ("int", "uint", "str", "time") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT)
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-52 b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-52
@@ -1 +1 @@
-INSERT INTO "models" ("int", "uint", "str", "time") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT)
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-165 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-165
@@ -0,0 +1 @@
+UPDATE `accounts` SET `is_active` = FALSE WHERE (`accounts`.`id` = 1)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-166 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-166
@@ -0,0 +1 @@
+INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-51 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-51
@@ -1 +1 @@
-INSERT INTO `models` (`int`, `uint`, `str`, `time`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT)
+INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-52 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-52
@@ -1 +1 @@
-INSERT INTO `models` (`int`, `uint`, `str`, `time`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT)
+INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-165 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-165
@@ -0,0 +1 @@
+UPDATE `accounts` SET `is_active` = FALSE WHERE (`accounts`.`id` = 1)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-166 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-166
@@ -0,0 +1 @@
+INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-51 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-51
@@ -1 +1 @@
-INSERT INTO `models` (`int`, `uint`, `str`, `time`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT)
+INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-52 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-52
@@ -1 +1 @@
-INSERT INTO `models` (`int`, `uint`, `str`, `time`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT)
+INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-165 b/internal/dbtest/testdata/snapshots/TestQuery-pg-165
@@ -0,0 +1 @@
+UPDATE "accounts" AS "accounts" SET "is_active" = FALSE WHERE ("accounts"."id" = 1)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-166 b/internal/dbtest/testdata/snapshots/TestQuery-pg-166
@@ -0,0 +1 @@
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING "int", "uint", "str", "time", "bool", "empty_str"
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-51 b/internal/dbtest/testdata/snapshots/TestQuery-pg-51
@@ -1 +1 @@
-INSERT INTO "models" ("int", "uint", "str", "time") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING "int", "uint", "str", "time"
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING "int", "uint", "str", "time", "bool", "empty_str"
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-52 b/internal/dbtest/testdata/snapshots/TestQuery-pg-52
@@ -1 +1 @@
-INSERT INTO "models" ("int", "uint", "str", "time") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING "int", "uint", "str", "time"
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING "int", "uint", "str", "time", "bool", "empty_str"
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-165 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-165
@@ -0,0 +1 @@
+UPDATE "accounts" AS "accounts" SET "is_active" = FALSE WHERE ("accounts"."id" = 1)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-166 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-166
@@ -0,0 +1 @@
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING "int", "uint", "str", "time", "bool", "empty_str"
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-51 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-51
@@ -1 +1 @@
-INSERT INTO "models" ("int", "uint", "str", "time") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING "int", "uint", "str", "time"
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING "int", "uint", "str", "time", "bool", "empty_str"
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-52 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-52
@@ -1 +1 @@
-INSERT INTO "models" ("int", "uint", "str", "time") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING "int", "uint", "str", "time"
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING "int", "uint", "str", "time", "bool", "empty_str"
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-165 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-165
@@ -0,0 +1 @@
+UPDATE "accounts" AS "accounts" SET "is_active" = FALSE WHERE ("accounts"."id" = 1)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-166 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-166
@@ -0,0 +1 @@
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (42, 42, 'hello', now(), true, '') RETURNING "int", "uint", "str", "time", "bool", "empty_str"
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-51 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-51
@@ -1 +1 @@
-INSERT INTO "models" ("int", "uint", "str", "time") VALUES (NULL, NULL, NULL, NULL) RETURNING "int", "uint", "str", "time"
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (NULL, NULL, NULL, NULL, NULL, NULL) RETURNING "int", "uint", "str", "time", "bool", "empty_str"
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-52 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-52
@@ -1 +1 @@
-INSERT INTO "models" ("int", "uint", "str", "time") VALUES (42, 42, 'hello', now()) RETURNING "int", "uint", "str", "time"
+INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (42, 42, 'hello', now(), true, '') RETURNING "int", "uint", "str", "time", "bool", "empty_str"
diff --git a/query_insert.go b/query_insert.go
@@ -332,8 +332,8 @@ func (q *InsertQuery) appendStructValues(
 		switch {
 		case isTemplate:
 			b = append(b, '?')
-		case (f.IsPtr && f.HasNilValue(strct)) || (f.NullZero && f.HasZeroValue(strct)):
-			if q.db.features.Has(feature.DefaultPlaceholder) {
+		case q.marshalsToDefault(f, strct):
+			if q.db.HasFeature(feature.DefaultPlaceholder) {
 				b = append(b, "DEFAULT"...)
 			} else if f.SQLDefault != "" {
 				b = append(b, f.SQLDefault...)
@@ -410,18 +410,23 @@ func (q *InsertQuery) getFields() ([]*schema.Field, error) {
 			q.addReturningField(f)
 			continue
 		}
-		if f.NotNull && f.SQLDefault == "" {
-			if (f.IsPtr && f.HasNilValue(strct)) || (f.NullZero && f.HasZeroValue(strct)) {
-				q.addReturningField(f)
-				continue
-			}
+		if f.NotNull && q.marshalsToDefault(f, strct) {
+			q.addReturningField(f)
+			continue
 		}
 		fields = append(fields, f)
 	}
 
 	return fields, nil
 }
 
+// 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 {
+	return (f.IsPtr && f.HasNilValue(v)) ||
+		(f.HasZeroValue(v) && (f.NullZero || f.SQLDefault != ""))
+}
+
 func (q *InsertQuery) appendFields(
 	fmter schema.Formatter, b []byte, fields []*schema.Field,
 ) []byte {

diff --git a/schema/table.go b/schema/table.go
@@ -377,7 +377,6 @@ func (t *Table) newField(f reflect.StructField, prefix string, index []int) *Fie
 	}
 	if s, ok := tag.Option("default"); ok {
 		field.SQLDefault = s
-		field.NullZero = true
 	}
 	if s, ok := field.Tag.Option("type"); ok {
 		field.UserSQLType = s
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		UPDATE `accounts` SET `is_active` = FALSE WHERE (`accounts`.`id` = 1)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING `int`, `uint`, `str`, `time`, `bool`, `empty_str`
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		INSERT INTO `models` (`int`, `uint`, `str`, `time`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING `int`, `uint`, `str`, `time`
		INSERT INTO `models` (`int`, `uint`, `str`, `time`, `bool`, `empty_str`) VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING `int`, `uint`, `str`, `time`, `bool`, `empty_str`
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		UPDATE "accounts" SET "is_active" = 0 WHERE ("id" = 1)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		INSERT INTO "models" ("int", "uint", "str", "time", "bool", "empty_str") VALUES (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		UPDATE "accounts" AS "accounts" SET "is_active" = FALSE WHERE ("accounts"."id" = 1)