From df1fb619ada3dac65a6c7d3ed715342e91b862ac Mon Sep 17 00:00:00 2001 From: John Guo Date: Fri, 13 Sep 2024 15:26:33 +0800 Subject: [PATCH 1/2] fix(database/gdb): #3754 FieldsEx feature conflicts with soft time feature in soft time fields updating --- contrib/drivers/mysql/testdata/issue3754.sql | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 contrib/drivers/mysql/testdata/issue3754.sql diff --git a/contrib/drivers/mysql/testdata/issue3754.sql b/contrib/drivers/mysql/testdata/issue3754.sql new file mode 100644 index 00000000000..93b5f9dacc9 --- /dev/null +++ b/contrib/drivers/mysql/testdata/issue3754.sql @@ -0,0 +1,14 @@ +CREATE TABLE `issue3218_sys_config` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `name` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NULL DEFAULT NULL COMMENT '配置名称', + `value` text CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NULL COMMENT '配置值', + `created_at` timestamp NULL DEFAULT NULL COMMENT '创建时间', + `updated_at` timestamp NULL DEFAULT NULL COMMENT '更新时间', + PRIMARY KEY (`id`) USING BTREE, + UNIQUE INDEX `name`(`name`(191)) USING BTREE +) ENGINE = InnoDB CHARACTER SET = utf8mb4 COLLATE = utf8mb4_general_ci; + +-- ---------------------------- +-- Records of sys_config +-- ---------------------------- +INSERT INTO `issue3218_sys_config` VALUES (49, 'site', '{\"banned_ip\":\"22\",\"filings\":\"2222\",\"fixed_page\":\"\",\"site_name\":\"22\",\"version\":\"22\"}', '2023-12-19 14:08:25', '2023-12-19 14:08:25'); \ No newline at end of file From a424fdf81a6193dad2ba19dfd305e064f7fe6bc8 Mon Sep 17 00:00:00 2001 From: John Guo Date: Fri, 13 Sep 2024 15:27:49 +0800 Subject: [PATCH 2/2] up --- .../drivers/mysql/mysql_z_unit_issue_test.go | 62 +++++++++++++ contrib/drivers/mysql/testdata/issue3754.sql | 22 ++--- database/gdb/gdb_model.go | 10 ++- database/gdb/gdb_model_fields.go | 88 ++++++++++++------- database/gdb/gdb_model_insert.go | 10 ++- database/gdb/gdb_model_select.go | 10 +-- database/gdb/gdb_model_update.go | 2 +- database/gdb/gdb_model_utility.go | 2 +- 8 files changed, 152 insertions(+), 54 deletions(-) diff --git a/contrib/drivers/mysql/mysql_z_unit_issue_test.go b/contrib/drivers/mysql/mysql_z_unit_issue_test.go index 0ccc54ab3d1..a2a7ce82c40 100644 --- a/contrib/drivers/mysql/mysql_z_unit_issue_test.go +++ b/contrib/drivers/mysql/mysql_z_unit_issue_test.go @@ -1216,3 +1216,65 @@ func Test_Issue3649(t *testing.T) { t.Assert(sql[0], sqlStr) }) } + +// https://github.com/gogf/gf/issues/3754 +func Test_Issue3754(t *testing.T) { + table := "issue3754" + array := gstr.SplitAndTrim(gtest.DataContent(`issue3754.sql`), ";") + for _, v := range array { + if _, err := db.Exec(ctx, v); err != nil { + gtest.Error(err) + } + } + defer dropTable(table) + + gtest.C(t, func(t *gtest.T) { + fieldsEx := []string{"delete_at", "create_at", "update_at"} + // Insert. + dataInsert := g.Map{ + "id": 1, + "name": "name_1", + } + r, err := db.Model(table).Data(dataInsert).FieldsEx(fieldsEx).Insert() + t.AssertNil(err) + n, _ := r.RowsAffected() + t.Assert(n, 1) + + oneInsert, err := db.Model(table).WherePri(1).One() + t.AssertNil(err) + t.Assert(oneInsert["id"].Int(), 1) + t.Assert(oneInsert["name"].String(), "name_1") + t.Assert(oneInsert["delete_at"].String(), "") + t.Assert(oneInsert["create_at"].String(), "") + t.Assert(oneInsert["update_at"].String(), "") + + // Update. + dataUpdate := g.Map{ + "name": "name_1000", + } + r, err = db.Model(table).Data(dataUpdate).FieldsEx(fieldsEx).WherePri(1).Update() + t.AssertNil(err) + n, _ = r.RowsAffected() + t.Assert(n, 1) + + oneUpdate, err := db.Model(table).WherePri(1).One() + t.AssertNil(err) + t.Assert(oneUpdate["id"].Int(), 1) + t.Assert(oneUpdate["name"].String(), "name_1000") + t.Assert(oneUpdate["delete_at"].String(), "") + t.Assert(oneUpdate["create_at"].String(), "") + t.Assert(oneUpdate["update_at"].String(), "") + + // FieldsEx does not affect Delete operation. + r, err = db.Model(table).FieldsEx(fieldsEx).WherePri(1).Delete() + n, _ = r.RowsAffected() + t.Assert(n, 1) + oneDeleteUnscoped, err := db.Model(table).Unscoped().WherePri(1).One() + t.AssertNil(err) + t.Assert(oneDeleteUnscoped["id"].Int(), 1) + t.Assert(oneDeleteUnscoped["name"].String(), "name_1000") + t.AssertNE(oneDeleteUnscoped["delete_at"].String(), "") + t.Assert(oneDeleteUnscoped["create_at"].String(), "") + t.Assert(oneDeleteUnscoped["update_at"].String(), "") + }) +} diff --git a/contrib/drivers/mysql/testdata/issue3754.sql b/contrib/drivers/mysql/testdata/issue3754.sql index 93b5f9dacc9..e6cdac030fc 100644 --- a/contrib/drivers/mysql/testdata/issue3754.sql +++ b/contrib/drivers/mysql/testdata/issue3754.sql @@ -1,14 +1,8 @@ -CREATE TABLE `issue3218_sys_config` ( - `id` int(11) NOT NULL AUTO_INCREMENT, - `name` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NULL DEFAULT NULL COMMENT '配置名称', - `value` text CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NULL COMMENT '配置值', - `created_at` timestamp NULL DEFAULT NULL COMMENT '创建时间', - `updated_at` timestamp NULL DEFAULT NULL COMMENT '更新时间', - PRIMARY KEY (`id`) USING BTREE, - UNIQUE INDEX `name`(`name`(191)) USING BTREE -) ENGINE = InnoDB CHARACTER SET = utf8mb4 COLLATE = utf8mb4_general_ci; - --- ---------------------------- --- Records of sys_config --- ---------------------------- -INSERT INTO `issue3218_sys_config` VALUES (49, 'site', '{\"banned_ip\":\"22\",\"filings\":\"2222\",\"fixed_page\":\"\",\"site_name\":\"22\",\"version\":\"22\"}', '2023-12-19 14:08:25', '2023-12-19 14:08:25'); \ No newline at end of file +CREATE TABLE `issue3754` ( + id int(11) NOT NULL, + name varchar(45) DEFAULT NULL, + create_at datetime(0) DEFAULT NULL, + update_at datetime(0) DEFAULT NULL, + delete_at datetime(0) DEFAULT NULL, + PRIMARY KEY (id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8; \ No newline at end of file diff --git a/database/gdb/gdb_model.go b/database/gdb/gdb_model.go index ab9cf4317a1..d2c812ccbe4 100644 --- a/database/gdb/gdb_model.go +++ b/database/gdb/gdb_model.go @@ -25,7 +25,7 @@ type Model struct { tablesInit string // Table names when model initialization. tables string // Operation table names, which can be more than one table names and aliases, like: "user", "user u", "user u, user_detail ud". fields string // Operation fields, multiple fields joined using char ','. - fieldsEx string // Excluded operation fields, multiple fields joined using char ','. + fieldsEx []string // Excluded operation fields, it here uses slice instead of string type for quick filtering. withArray []interface{} // Arguments for With feature. withAll bool // Enable model association operations on all objects that have "with" tag in the struct. extraArgs []interface{} // Extra custom arguments for sql, which are prepended to the arguments before sql committed to underlying driver. @@ -281,6 +281,10 @@ func (m *Model) Clone() *Model { newModel.whereBuilder = m.whereBuilder.Clone() newModel.whereBuilder.model = newModel // Shallow copy slice attributes. + if n := len(m.fieldsEx); n > 0 { + newModel.fieldsEx = make([]string, n) + copy(newModel.fieldsEx, m.fieldsEx) + } if n := len(m.extraArgs); n > 0 { newModel.extraArgs = make([]interface{}, n) copy(newModel.extraArgs, m.extraArgs) @@ -289,6 +293,10 @@ func (m *Model) Clone() *Model { newModel.withArray = make([]interface{}, n) copy(newModel.withArray, m.withArray) } + if n := len(m.having); n > 0 { + newModel.having = make([]interface{}, n) + copy(newModel.having, m.having) + } return newModel } diff --git a/database/gdb/gdb_model_fields.go b/database/gdb/gdb_model_fields.go index c9e29a3f89d..1615090daf5 100644 --- a/database/gdb/gdb_model_fields.go +++ b/database/gdb/gdb_model_fields.go @@ -10,6 +10,7 @@ import ( "fmt" "github.com/gogf/gf/v2/container/gset" + "github.com/gogf/gf/v2/errors/gerror" "github.com/gogf/gf/v2/text/gstr" "github.com/gogf/gf/v2/util/gconv" ) @@ -17,31 +18,36 @@ import ( // Fields appends `fieldNamesOrMapStruct` to the operation fields of the model, multiple fields joined using char ','. // The parameter `fieldNamesOrMapStruct` can be type of string/map/*map/struct/*struct. // -// Eg: +// Example: // Fields("id", "name", "age") // Fields([]string{"id", "name", "age"}) // Fields(map[string]interface{}{"id":1, "name":"john", "age":18}) -// Fields(User{ Id: 1, Name: "john", Age: 18}). +// Fields(User{Id: 1, Name: "john", Age: 18}). func (m *Model) Fields(fieldNamesOrMapStruct ...interface{}) *Model { length := len(fieldNamesOrMapStruct) if length == 0 { return m } - fields := m.getFieldsFrom(m.tablesInit, fieldNamesOrMapStruct...) + fields := m.filterFieldsFrom(m.tablesInit, fieldNamesOrMapStruct...) if len(fields) == 0 { return m } - return m.appendFieldsByStr(gstr.Join(fields, ",")) + model := m.getModel() + return model.appendFieldsByStr(gstr.Join(fields, ",")) } // FieldsPrefix performs as function Fields but add extra prefix for each field. func (m *Model) FieldsPrefix(prefixOrAlias string, fieldNamesOrMapStruct ...interface{}) *Model { - fields := m.getFieldsFrom(m.getTableNameByPrefixOrAlias(prefixOrAlias), fieldNamesOrMapStruct...) + fields := m.filterFieldsFrom( + m.getTableNameByPrefixOrAlias(prefixOrAlias), + fieldNamesOrMapStruct..., + ) if len(fields) == 0 { return m } gstr.PrefixArray(fields, prefixOrAlias+".") - return m.appendFieldsByStr(gstr.Join(fields, ",")) + model := m.getModel() + return model.appendFieldsByStr(gstr.Join(fields, ",")) } // FieldsEx appends `fieldNamesOrMapStruct` to the excluded operation fields of the model, @@ -49,28 +55,36 @@ func (m *Model) FieldsPrefix(prefixOrAlias string, fieldNamesOrMapStruct ...inte // Note that this function supports only single table operations. // The parameter `fieldNamesOrMapStruct` can be type of string/map/*map/struct/*struct. // -// Also see Fields. +// Example: +// FieldsEx("id", "name", "age") +// FieldsEx([]string{"id", "name", "age"}) +// FieldsEx(map[string]interface{}{"id":1, "name":"john", "age":18}) +// FieldsEx(User{Id: 1, Name: "john", Age: 18}). func (m *Model) FieldsEx(fieldNamesOrMapStruct ...interface{}) *Model { return m.doFieldsEx(m.tablesInit, fieldNamesOrMapStruct...) } + func (m *Model) doFieldsEx(table string, fieldNamesOrMapStruct ...interface{}) *Model { length := len(fieldNamesOrMapStruct) if length == 0 { return m } - fields := m.getFieldsFrom(table, fieldNamesOrMapStruct...) + fields := m.filterFieldsFrom(table, fieldNamesOrMapStruct...) if len(fields) == 0 { return m } - return m.appendFieldsExByStr(gstr.Join(fields, ",")) + model := m.getModel() + model.fieldsEx = append(model.fieldsEx, fields...) + return model } // FieldsExPrefix performs as function FieldsEx but add extra prefix for each field. func (m *Model) FieldsExPrefix(prefixOrAlias string, fieldNamesOrMapStruct ...interface{}) *Model { - model := m.doFieldsEx(m.getTableNameByPrefixOrAlias(prefixOrAlias), fieldNamesOrMapStruct...) - array := gstr.SplitAndTrim(model.fieldsEx, ",") - gstr.PrefixArray(array, prefixOrAlias+".") - model.fieldsEx = gstr.Join(array, ",") + model := m.doFieldsEx( + m.getTableNameByPrefixOrAlias(prefixOrAlias), + fieldNamesOrMapStruct..., + ) + gstr.PrefixArray(model.fieldsEx, prefixOrAlias+".") return model } @@ -80,7 +94,10 @@ func (m *Model) FieldCount(column string, as ...string) *Model { if len(as) > 0 && as[0] != "" { asStr = fmt.Sprintf(` AS %s`, m.db.GetCore().QuoteWord(as[0])) } - return m.appendFieldsByStr(fmt.Sprintf(`COUNT(%s)%s`, m.QuoteWord(column), asStr)) + model := m.getModel() + return model.appendFieldsByStr( + fmt.Sprintf(`COUNT(%s)%s`, m.QuoteWord(column), asStr), + ) } // FieldSum formats and appends commonly used field `SUM(column)` to the select fields of model. @@ -89,7 +106,10 @@ func (m *Model) FieldSum(column string, as ...string) *Model { if len(as) > 0 && as[0] != "" { asStr = fmt.Sprintf(` AS %s`, m.db.GetCore().QuoteWord(as[0])) } - return m.appendFieldsByStr(fmt.Sprintf(`SUM(%s)%s`, m.QuoteWord(column), asStr)) + model := m.getModel() + return model.appendFieldsByStr( + fmt.Sprintf(`SUM(%s)%s`, m.QuoteWord(column), asStr), + ) } // FieldMin formats and appends commonly used field `MIN(column)` to the select fields of model. @@ -98,7 +118,10 @@ func (m *Model) FieldMin(column string, as ...string) *Model { if len(as) > 0 && as[0] != "" { asStr = fmt.Sprintf(` AS %s`, m.db.GetCore().QuoteWord(as[0])) } - return m.appendFieldsByStr(fmt.Sprintf(`MIN(%s)%s`, m.QuoteWord(column), asStr)) + model := m.getModel() + return model.appendFieldsByStr( + fmt.Sprintf(`MIN(%s)%s`, m.QuoteWord(column), asStr), + ) } // FieldMax formats and appends commonly used field `MAX(column)` to the select fields of model. @@ -107,7 +130,10 @@ func (m *Model) FieldMax(column string, as ...string) *Model { if len(as) > 0 && as[0] != "" { asStr = fmt.Sprintf(` AS %s`, m.db.GetCore().QuoteWord(as[0])) } - return m.appendFieldsByStr(fmt.Sprintf(`MAX(%s)%s`, m.QuoteWord(column), asStr)) + model := m.getModel() + return model.appendFieldsByStr( + fmt.Sprintf(`MAX(%s)%s`, m.QuoteWord(column), asStr), + ) } // FieldAvg formats and appends commonly used field `AVG(column)` to the select fields of model. @@ -116,7 +142,10 @@ func (m *Model) FieldAvg(column string, as ...string) *Model { if len(as) > 0 && as[0] != "" { asStr = fmt.Sprintf(` AS %s`, m.db.GetCore().QuoteWord(as[0])) } - return m.appendFieldsByStr(fmt.Sprintf(`AVG(%s)%s`, m.QuoteWord(column), asStr)) + model := m.getModel() + return model.appendFieldsByStr( + fmt.Sprintf(`AVG(%s)%s`, m.QuoteWord(column), asStr), + ) } // GetFieldsStr retrieves and returns all fields from the table, joined with char ','. @@ -152,17 +181,17 @@ func (m *Model) GetFieldsStr(prefix ...string) string { // joined with char ','. // The parameter `fields` specifies the fields that are excluded. // The optional parameter `prefix` specifies the prefix for each field, eg: FieldsExStr("id", "u."). -func (m *Model) GetFieldsExStr(fields string, prefix ...string) string { +func (m *Model) GetFieldsExStr(fields string, prefix ...string) (string, error) { prefixStr := "" if len(prefix) > 0 { prefixStr = prefix[0] } tableFields, err := m.TableFields(m.tablesInit) if err != nil { - panic(err) + return "", err } if len(tableFields) == 0 { - panic(fmt.Sprintf(`empty table fields for table "%s"`, m.tables)) + return "", gerror.Newf(`empty table fields for table "%s"`, m.tables) } fieldsExSet := gset.NewStrSetFrom(gstr.SplitAndTrim(fields, ",")) fieldsArray := make([]string, len(tableFields)) @@ -180,7 +209,7 @@ func (m *Model) GetFieldsExStr(fields string, prefix ...string) string { newFields += prefixStr + k } newFields = m.db.GetCore().QuoteString(newFields) - return newFields + return newFields, nil } // HasField determine whether the field exists in the table. @@ -189,7 +218,7 @@ func (m *Model) HasField(field string) (bool, error) { } // getFieldsFrom retrieves, filters and returns fields name from table `table`. -func (m *Model) getFieldsFrom(table string, fieldNamesOrMapStruct ...interface{}) []string { +func (m *Model) filterFieldsFrom(table string, fieldNamesOrMapStruct ...interface{}) []string { length := len(fieldNamesOrMapStruct) if length == 0 { return nil @@ -238,14 +267,11 @@ func (m *Model) appendFieldsByStr(fields string) *Model { return m } -func (m *Model) appendFieldsExByStr(fieldsEx string) *Model { - if fieldsEx != "" { - model := m.getModel() - if model.fieldsEx != "" { - model.fieldsEx += "," +func (m *Model) isFieldInFieldsEx(field string) bool { + for _, v := range m.fieldsEx { + if v == field { + return true } - model.fieldsEx += fieldsEx - return model } - return m + return false } diff --git a/database/gdb/gdb_model_insert.go b/database/gdb/gdb_model_insert.go index 798b355ae11..753677c994f 100644 --- a/database/gdb/gdb_model_insert.go +++ b/database/gdb/gdb_model_insert.go @@ -285,7 +285,14 @@ func (m *Model) doInsertWithOption(ctx context.Context, insertOption InsertOptio } // Automatic handling for creating/updating time. - if !m.unscoped && (fieldNameCreate != "" || fieldNameUpdate != "") { + if fieldNameCreate != "" && m.isFieldInFieldsEx(fieldNameCreate) { + fieldNameCreate = "" + } + if fieldNameUpdate != "" && m.isFieldInFieldsEx(fieldNameUpdate) { + fieldNameUpdate = "" + } + var isSoftTimeFeatureEnabled = fieldNameCreate != "" || fieldNameUpdate != "" + if !m.unscoped && isSoftTimeFeatureEnabled { for k, v := range list { if fieldNameCreate != "" && empty.IsNil(v[fieldNameCreate]) { fieldCreateValue := stm.GetValueByFieldTypeForCreateOrUpdate(ctx, fieldTypeCreate, false) @@ -299,6 +306,7 @@ func (m *Model) doInsertWithOption(ctx context.Context, insertOption InsertOptio v[fieldNameUpdate] = fieldUpdateValue } } + // for timestamp field that should initialize the delete_at field with value, for example 0. if fieldNameDelete != "" && empty.IsNil(v[fieldNameDelete]) { fieldDeleteValue := stm.GetValueByFieldTypeForCreateOrUpdate(ctx, fieldTypeDelete, true) if fieldDeleteValue != nil { diff --git a/database/gdb/gdb_model_select.go b/database/gdb/gdb_model_select.go index 975ca6dc41d..1b15e611a52 100644 --- a/database/gdb/gdb_model_select.go +++ b/database/gdb/gdb_model_select.go @@ -176,7 +176,7 @@ func (m *Model) Array(fieldsAndWhere ...interface{}) ([]Value, error) { func (m *Model) doStruct(pointer interface{}, where ...interface{}) error { model := m // Auto selecting fields by struct attributes. - if model.fieldsEx == "" && (model.fields == "" || model.fields == "*") { + if len(model.fieldsEx) == 0 && (model.fields == "" || model.fields == "*") { if v, ok := pointer.(reflect.Value); ok { model = m.Fields(v.Interface()) } else { @@ -212,7 +212,7 @@ func (m *Model) doStruct(pointer interface{}, where ...interface{}) error { func (m *Model) doStructs(pointer interface{}, where ...interface{}) error { model := m // Auto selecting fields by struct attributes. - if model.fieldsEx == "" && (model.fields == "" || model.fields == "*") { + if len(model.fieldsEx) == 0 && (model.fields == "" || model.fields == "*") { if v, ok := pointer.(reflect.Value); ok { model = m.Fields( reflect.New( @@ -341,7 +341,7 @@ func (m *Model) ScanList(structSlicePointer interface{}, bindToAttrName string, if err != nil { return err } - if m.fields != defaultFields || m.fieldsEx != "" { + if m.fields != defaultFields || len(m.fieldsEx) != 0 { // There are custom fields. result, err = m.All() } else { @@ -675,7 +675,7 @@ func (m *Model) getAutoPrefix() string { // getFieldsFiltered checks the fields and fieldsEx attributes, filters and returns the fields that will // really be committed to underlying database driver. func (m *Model) getFieldsFiltered() string { - if m.fieldsEx == "" { + if len(m.fieldsEx) == 0 { // No filtering, containing special chars. if gstr.ContainsAny(m.fields, "()") { return m.fields @@ -688,7 +688,7 @@ func (m *Model) getFieldsFiltered() string { } var ( fieldsArray []string - fieldsExSet = gset.NewStrSetFrom(gstr.SplitAndTrim(m.fieldsEx, ",")) + fieldsExSet = gset.NewStrSetFrom(m.fieldsEx) ) if m.fields != "*" { // Filter custom fields with fieldEx. diff --git a/database/gdb/gdb_model_update.go b/database/gdb/gdb_model_update.go index 2d14e558db8..44c04d8e67d 100644 --- a/database/gdb/gdb_model_update.go +++ b/database/gdb/gdb_model_update.go @@ -54,7 +54,7 @@ func (m *Model) Update(dataAndWhere ...interface{}) (result sql.Result, err erro ctx, "", m.tablesInit, ) ) - if m.unscoped { + if fieldNameUpdate != "" && (m.unscoped || m.isFieldInFieldsEx(fieldNameUpdate)) { fieldNameUpdate = "" } diff --git a/database/gdb/gdb_model_utility.go b/database/gdb/gdb_model_utility.go index 9a5da85225e..1b8d3704f81 100644 --- a/database/gdb/gdb_model_utility.go +++ b/database/gdb/gdb_model_utility.go @@ -204,7 +204,7 @@ func (m *Model) doMappingAndFilterForInsertOrUpdateDataMap(data Map, allowOmitEm } } else if len(m.fieldsEx) > 0 { // Filter specified fields. - for _, v := range gstr.SplitAndTrim(m.fieldsEx, ",") { + for _, v := range m.fieldsEx { delete(data, v) } }