From b398e6bea840ea2fd3e001b7879c0b00b6dcd6f7 Mon Sep 17 00:00:00 2001 From: Vladimir Mihailenco Date: Wed, 2 Mar 2022 09:45:06 +0200 Subject: [PATCH] feat: improve nil ptr values handling --- dialect/pgdialect/sqltype.go | 1 - internal/dbtest/query_test.go | 15 +++++++++- .../testdata/snapshots/TestQuery-mariadb-109 | 1 + .../testdata/snapshots/TestQuery-mariadb-110 | 1 + .../testdata/snapshots/TestQuery-mariadb-59 | 2 +- .../snapshots/TestQuery-mssql2019-109 | 1 + .../snapshots/TestQuery-mssql2019-110 | 1 + .../testdata/snapshots/TestQuery-mssql2019-59 | 2 +- .../testdata/snapshots/TestQuery-mysql5-109 | 1 + .../testdata/snapshots/TestQuery-mysql5-110 | 1 + .../testdata/snapshots/TestQuery-mysql5-59 | 2 +- .../testdata/snapshots/TestQuery-mysql8-109 | 1 + .../testdata/snapshots/TestQuery-mysql8-110 | 1 + .../testdata/snapshots/TestQuery-mysql8-59 | 2 +- .../testdata/snapshots/TestQuery-pg-109 | 1 + .../testdata/snapshots/TestQuery-pg-110 | 1 + .../dbtest/testdata/snapshots/TestQuery-pg-59 | 2 +- .../testdata/snapshots/TestQuery-pgx-109 | 1 + .../testdata/snapshots/TestQuery-pgx-110 | 1 + .../testdata/snapshots/TestQuery-pgx-59 | 2 +- .../testdata/snapshots/TestQuery-sqlite-109 | 1 + .../testdata/snapshots/TestQuery-sqlite-110 | 1 + .../testdata/snapshots/TestQuery-sqlite-59 | 2 +- query_base.go | 2 +- query_insert.go | 10 ++++--- schema/append_value.go | 4 ++- schema/field.go | 28 +++++++++++++++++-- schema/reflect.go | 3 +- schema/table.go | 1 + 29 files changed, 73 insertions(+), 19 deletions(-) create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mariadb-109 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mariadb-110 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mssql2019-109 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mssql2019-110 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mysql5-109 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mysql5-110 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mysql8-109 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mysql8-110 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-pg-109 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-pg-110 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-pgx-109 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-pgx-110 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-sqlite-109 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-sqlite-110 diff --git a/dialect/pgdialect/sqltype.go b/dialect/pgdialect/sqltype.go index 1fbfa7d7f..bfef89fa1 100644 --- a/dialect/pgdialect/sqltype.go +++ b/dialect/pgdialect/sqltype.go @@ -53,7 +53,6 @@ func fieldSQLType(field *schema.Field) string { if v, ok := field.Tag.Option("composite"); ok { return v } - if _, ok := field.Tag.Option("hstore"); ok { return "hstore" } diff --git a/internal/dbtest/query_test.go b/internal/dbtest/query_test.go index e6665e799..7a905af0d 100644 --- a/internal/dbtest/query_test.go +++ b/internal/dbtest/query_test.go @@ -658,7 +658,7 @@ func TestQuery(t *testing.T) { }, func(db *bun.DB) schema.QueryAppender { type Model struct { - Raw *json.RawMessage `bun:",nullzero"` + Raw *json.RawMessage } return db.NewInsert().Model(new(Model)) }, @@ -682,6 +682,19 @@ func TestQuery(t *testing.T) { } return db.NewInsert().Model(&Model{ID: 123, Slice: make([]Item, 0)}) }, + func(db *bun.DB) schema.QueryAppender { + type Model struct { + Time *time.Time + } + return db.NewInsert().Model(new(Model)) + }, + func(db *bun.DB) schema.QueryAppender { + type Model struct { + Time *time.Time + } + tm := time.Unix(0, 0) + return db.NewInsert().Model(&Model{Time: &tm}) + }, } 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-109 b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-109 new file mode 100644 index 000000000..e351c8ca0 --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-109 @@ -0,0 +1 @@ +INSERT INTO `models` (`time`) VALUES (DEFAULT) RETURNING `time` diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-110 b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-110 new file mode 100644 index 000000000..d726e0ea3 --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-110 @@ -0,0 +1 @@ +INSERT INTO `models` (`time`) VALUES ('1970-01-01 00:00:00') diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-59 b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-59 index 07c8a4f6d..bc6a3a096 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-59 +++ b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-59 @@ -1 +1 @@ -INSERT INTO `models` (`raw`) VALUES ('null') +INSERT INTO `models` (`raw`) VALUES (DEFAULT) RETURNING `raw` diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-109 b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-109 new file mode 100644 index 000000000..f663766ae --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-109 @@ -0,0 +1 @@ +INSERT INTO "models" ("time") VALUES (DEFAULT) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-110 b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-110 new file mode 100644 index 000000000..780810d75 --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-110 @@ -0,0 +1 @@ +INSERT INTO "models" ("time") VALUES ('1970-01-01 00:00:00') diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-59 b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-59 index 0e7b3b827..202e3b0a1 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-59 +++ b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-59 @@ -1 +1 @@ -INSERT INTO "models" ("raw") VALUES ('null') +INSERT INTO "models" ("raw") VALUES (DEFAULT) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-109 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-109 new file mode 100644 index 000000000..a4dfe4b39 --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-109 @@ -0,0 +1 @@ +INSERT INTO `models` (`time`) VALUES (DEFAULT) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-110 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-110 new file mode 100644 index 000000000..d726e0ea3 --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-110 @@ -0,0 +1 @@ +INSERT INTO `models` (`time`) VALUES ('1970-01-01 00:00:00') diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-59 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-59 index 07c8a4f6d..bef7d892e 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-59 +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-59 @@ -1 +1 @@ -INSERT INTO `models` (`raw`) VALUES ('null') +INSERT INTO `models` (`raw`) VALUES (DEFAULT) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-109 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-109 new file mode 100644 index 000000000..a4dfe4b39 --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-109 @@ -0,0 +1 @@ +INSERT INTO `models` (`time`) VALUES (DEFAULT) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-110 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-110 new file mode 100644 index 000000000..d726e0ea3 --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-110 @@ -0,0 +1 @@ +INSERT INTO `models` (`time`) VALUES ('1970-01-01 00:00:00') diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-59 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-59 index 07c8a4f6d..bef7d892e 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-59 +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-59 @@ -1 +1 @@ -INSERT INTO `models` (`raw`) VALUES ('null') +INSERT INTO `models` (`raw`) VALUES (DEFAULT) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-109 b/internal/dbtest/testdata/snapshots/TestQuery-pg-109 new file mode 100644 index 000000000..9e04fc0fc --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-pg-109 @@ -0,0 +1 @@ +INSERT INTO "models" ("time") VALUES (DEFAULT) RETURNING "time" diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-110 b/internal/dbtest/testdata/snapshots/TestQuery-pg-110 new file mode 100644 index 000000000..b25143c57 --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-pg-110 @@ -0,0 +1 @@ +INSERT INTO "models" ("time") VALUES ('1970-01-01 00:00:00+00:00') diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-59 b/internal/dbtest/testdata/snapshots/TestQuery-pg-59 index 0e7b3b827..ef36680da 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-pg-59 +++ b/internal/dbtest/testdata/snapshots/TestQuery-pg-59 @@ -1 +1 @@ -INSERT INTO "models" ("raw") VALUES ('null') +INSERT INTO "models" ("raw") VALUES (DEFAULT) RETURNING "raw" diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-109 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-109 new file mode 100644 index 000000000..9e04fc0fc --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-pgx-109 @@ -0,0 +1 @@ +INSERT INTO "models" ("time") VALUES (DEFAULT) RETURNING "time" diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-110 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-110 new file mode 100644 index 000000000..b25143c57 --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-pgx-110 @@ -0,0 +1 @@ +INSERT INTO "models" ("time") VALUES ('1970-01-01 00:00:00+00:00') diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-59 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-59 index 0e7b3b827..ef36680da 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-pgx-59 +++ b/internal/dbtest/testdata/snapshots/TestQuery-pgx-59 @@ -1 +1 @@ -INSERT INTO "models" ("raw") VALUES ('null') +INSERT INTO "models" ("raw") VALUES (DEFAULT) RETURNING "raw" diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-109 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-109 new file mode 100644 index 000000000..2c2163d9f --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-109 @@ -0,0 +1 @@ +INSERT INTO "models" ("time") VALUES (NULL) RETURNING "time" diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-110 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-110 new file mode 100644 index 000000000..b25143c57 --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-110 @@ -0,0 +1 @@ +INSERT INTO "models" ("time") VALUES ('1970-01-01 00:00:00+00:00') diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-59 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-59 index 0e7b3b827..f2c1073d4 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-59 +++ b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-59 @@ -1 +1 @@ -INSERT INTO "models" ("raw") VALUES ('null') +INSERT INTO "models" ("raw") VALUES (NULL) RETURNING "raw" diff --git a/query_base.go b/query_base.go index 1fe7899ec..d0ec0bfc6 100644 --- a/query_base.go +++ b/query_base.go @@ -780,7 +780,7 @@ func (q *whereBaseQuery) appendWhere( field := q.tableModel.Table().SoftDeleteField b = append(b, field.SQLName...) - if field.NullZero { + if field.IsPtr || field.NullZero { if q.flags.Has(deletedFlag) { b = append(b, " IS NOT NULL"...) } else { diff --git a/query_insert.go b/query_insert.go index ab57d77bd..fdbe0c275 100644 --- a/query_insert.go +++ b/query_insert.go @@ -319,7 +319,7 @@ func (q *InsertQuery) appendStructValues( switch { case isTemplate: b = append(b, '?') - case f.NullZero && f.HasZeroValue(strct): + case (f.IsPtr && f.HasNilValue(strct)) || (f.NullZero && f.HasZeroValue(strct)): if q.db.features.Has(feature.DefaultPlaceholder) { b = append(b, "DEFAULT"...) } else if f.SQLDefault != "" { @@ -397,9 +397,11 @@ func (q *InsertQuery) getFields() ([]*schema.Field, error) { q.addReturningField(f) continue } - if f.NotNull && f.NullZero && f.SQLDefault == "" && f.HasZeroValue(strct) { - q.addReturningField(f) - continue + if f.NotNull && f.SQLDefault == "" { + if (f.IsPtr && f.HasNilValue(strct)) || (f.NullZero && f.HasZeroValue(strct)) { + q.addReturningField(f) + continue + } } fields = append(fields, f) } diff --git a/schema/append_value.go b/schema/append_value.go index c805c745f..7e9c451db 100644 --- a/schema/append_value.go +++ b/schema/append_value.go @@ -97,6 +97,8 @@ func appender(dialect Dialect, typ reflect.Type) AppenderFunc { return appendBytesValue case timeType: return appendTimeValue + case timePtrType: + return PtrAppender(appendTimeValue) case ipType: return appendIPValue case ipNetType: @@ -135,7 +137,7 @@ func appender(dialect Dialect, typ reflect.Type) AppenderFunc { return ifaceAppenderFunc case reflect.Ptr: if typ.Implements(jsonMarshalerType) { - return AppendJSONValue + return nilAwareAppender(AppendJSONValue) } if fn := Appender(dialect, typ.Elem()); fn != nil { return PtrAppender(fn) diff --git a/schema/field.go b/schema/field.go index 8f4fba684..ac6359da4 100644 --- a/schema/field.go +++ b/schema/field.go @@ -10,6 +10,7 @@ import ( type Field struct { StructField reflect.StructField + IsPtr bool Tag tagparser.Tag IndirectType reflect.Type @@ -51,15 +52,36 @@ func (f *Field) Value(strct reflect.Value) reflect.Value { return fieldByIndexAlloc(strct, f.Index) } +func (f *Field) HasNilValue(v reflect.Value) bool { + if len(f.Index) == 1 { + return v.Field(f.Index[0]).IsNil() + } + + for _, index := range f.Index { + if v.Kind() == reflect.Ptr { + if v.IsNil() { + return true + } + v = v.Elem() + } + v = v.Field(index) + } + return v.IsNil() +} + func (f *Field) HasZeroValue(v reflect.Value) bool { - for _, idx := range f.Index { + if len(f.Index) == 1 { + return f.IsZero(v.Field(f.Index[0])) + } + + for _, index := range f.Index { if v.Kind() == reflect.Ptr { if v.IsNil() { return true } v = v.Elem() } - v = v.Field(idx) + v = v.Field(index) } return f.IsZero(v) } @@ -70,7 +92,7 @@ func (f *Field) AppendValue(fmter Formatter, b []byte, strct reflect.Value) []by return dialect.AppendNull(b) } - if f.NullZero && f.IsZero(fv) { + if (f.IsPtr && fv.IsNil()) || (f.NullZero && f.IsZero(fv)) { return dialect.AppendNull(b) } if f.Append == nil { diff --git a/schema/reflect.go b/schema/reflect.go index ff14f3bfa..f13826a6c 100644 --- a/schema/reflect.go +++ b/schema/reflect.go @@ -10,7 +10,8 @@ import ( var ( bytesType = reflect.TypeOf((*[]byte)(nil)).Elem() - timeType = reflect.TypeOf((*time.Time)(nil)).Elem() + timePtrType = reflect.TypeOf((*time.Time)(nil)) + timeType = timePtrType.Elem() ipType = reflect.TypeOf((*net.IP)(nil)).Elem() ipNetType = reflect.TypeOf((*net.IPNet)(nil)).Elem() jsonRawMessageType = reflect.TypeOf((*json.RawMessage)(nil)).Elem() diff --git a/schema/table.go b/schema/table.go index b79953aea..ef5f89c61 100644 --- a/schema/table.go +++ b/schema/table.go @@ -332,6 +332,7 @@ func (t *Table) newField(f reflect.StructField, prefix string, index []int) *Fie field := &Field{ StructField: f, + IsPtr: f.Type.Kind() == reflect.Ptr, Tag: tag, IndirectType: indirectType(f.Type),