From 2968370918f18abb1dfddad09cba2ebc41ee371d Mon Sep 17 00:00:00 2001 From: x <895740313@qq.com> Date: Tue, 1 Aug 2023 23:42:46 +0800 Subject: [PATCH 1/6] Fixed: The clickhouse time is automatically written, causing the modification operation to fail --- contrib/drivers/clickhouse/clickhouse_test.go | 21 +++++++++++++++++++ database/gdb/gdb_model_insert.go | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/contrib/drivers/clickhouse/clickhouse_test.go b/contrib/drivers/clickhouse/clickhouse_test.go index cc4f202ae70..2f57d9abdcf 100644 --- a/contrib/drivers/clickhouse/clickhouse_test.go +++ b/contrib/drivers/clickhouse/clickhouse_test.go @@ -263,6 +263,27 @@ func TestDriverClickhouse_InsertOne(t *testing.T) { gtest.AssertNil(err) } +func TestDriverClickhouse_InsertOneAutoDateTimeWrite(t *testing.T) { + connect, err := gdb.New(gdb.ConfigNode{ + Host: "127.0.0.1", + Port: "9000", + User: "default", + Name: "default", + Type: "clickhouse", + Debug: false, + CreatedAt: "created", + }) + gtest.AssertNil(err) + gtest.AssertNE(connect, nil) + gtest.AssertEQ(createClickhouseTableVisits(connect), nil) + defer dropClickhouseTableVisits(connect) + _, err = connect.Model("visits").Data(g.Map{ + "duration": float64(grand.Intn(999)), + "url": gconv.String(grand.Intn(999)), + }).Insert() + gtest.AssertNil(err) +} + func TestDriverClickhouse_InsertMany(t *testing.T) { connect := clickhouseConfigDB() gtest.AssertEQ(createClickhouseTableVisits(connect), nil) diff --git a/database/gdb/gdb_model_insert.go b/database/gdb/gdb_model_insert.go index 8318f08c8f1..2b3838a5ae4 100644 --- a/database/gdb/gdb_model_insert.go +++ b/database/gdb/gdb_model_insert.go @@ -260,7 +260,7 @@ func (m *Model) doInsertWithOption(ctx context.Context, insertOption InsertOptio } var ( list List - now = gtime.Now() + now = gtime.Now().Time fieldNameCreate = m.getSoftFieldNameCreated("", m.tablesInit) fieldNameUpdate = m.getSoftFieldNameUpdated("", m.tablesInit) ) From b05bac9a64ce61bb1461c68f1e1f342c36b65165 Mon Sep 17 00:00:00 2001 From: John Guo Date: Wed, 2 Aug 2023 22:02:12 +0800 Subject: [PATCH 2/6] improve data converting for DB.DoInsert/DoUpdate --- contrib/drivers/clickhouse/clickhouse.go | 23 ++++++--- database/gdb/gdb_core.go | 7 ++- database/gdb/gdb_driver_wrapper_db.go | 23 +++++++++ database/gdb/gdb_model_insert.go | 61 +++++------------------- database/gdb/gdb_model_update.go | 9 ++-- 5 files changed, 61 insertions(+), 62 deletions(-) diff --git a/contrib/drivers/clickhouse/clickhouse.go b/contrib/drivers/clickhouse/clickhouse.go index 195febcac9e..d5b4f99b6bc 100644 --- a/contrib/drivers/clickhouse/clickhouse.go +++ b/contrib/drivers/clickhouse/clickhouse.go @@ -18,6 +18,9 @@ import ( "time" "github.com/ClickHouse/clickhouse-go/v2" + "github.com/google/uuid" + "github.com/shopspring/decimal" + "github.com/gogf/gf/v2/database/gdb" "github.com/gogf/gf/v2/errors/gcode" "github.com/gogf/gf/v2/errors/gerror" @@ -27,8 +30,6 @@ import ( "github.com/gogf/gf/v2/util/gconv" "github.com/gogf/gf/v2/util/gtag" "github.com/gogf/gf/v2/util/gutil" - "github.com/google/uuid" - "github.com/shopspring/decimal" ) // Driver is the driver for postgresql database. @@ -298,13 +299,20 @@ func (d *Driver) DoInsert( keysStr = charL + strings.Join(keys, charR+","+charL) + charR holderStr = strings.Join(valueHolder, ",") tx gdb.TX - stdSqlResult sql.Result stmt *gdb.Stmt ) tx, err = d.Core.Begin(ctx) if err != nil { return } + // It here uses defer to guarantee transaction be committed or roll-backed. + defer func() { + if err == nil { + _ = tx.Commit() + } else { + _ = tx.Rollback() + } + }() stmt, err = tx.Prepare(fmt.Sprintf( "INSERT INTO %s(%s) VALUES (%s)", d.QuotePrefixTableName(table), keysStr, @@ -314,17 +322,18 @@ func (d *Driver) DoInsert( return } for i := 0; i < len(list); i++ { - params := make([]interface{}, 0) // Values that will be committed to underlying database driver. + // Values that will be committed to underlying database driver. + params := make([]interface{}, 0) for _, k := range keys { params = append(params, list[i][k]) } // Prepare is allowed to execute only once in a transaction opened by clickhouse - stdSqlResult, err = stmt.ExecContext(ctx, params...) + result, err = stmt.ExecContext(ctx, params...) if err != nil { - return stdSqlResult, err + return } } - return stdSqlResult, tx.Commit() + return } // ConvertDataForRecord converting for any data that will be inserted into table/collection as a record. diff --git a/database/gdb/gdb_core.go b/database/gdb/gdb_core.go index 7e217367867..8667abf6cbf 100644 --- a/database/gdb/gdb_core.go +++ b/database/gdb/gdb_core.go @@ -684,7 +684,12 @@ func (c *Core) DoUpdate(ctx context.Context, link Link, table string, data inter return nil, err } } - return c.db.DoExec(ctx, link, fmt.Sprintf("UPDATE %s SET %s%s", table, updates, condition), args...) + return c.db.DoExec(ctx, link, fmt.Sprintf( + "UPDATE %s SET %s%s", + table, updates, condition, + ), + args..., + ) } // Delete does "DELETE FROM ... " statement for the table. diff --git a/database/gdb/gdb_driver_wrapper_db.go b/database/gdb/gdb_driver_wrapper_db.go index 8605c800aa2..da590793b90 100644 --- a/database/gdb/gdb_driver_wrapper_db.go +++ b/database/gdb/gdb_driver_wrapper_db.go @@ -89,3 +89,26 @@ func (d *DriverWrapperDB) TableFields( } return } + +// DoInsert inserts or updates data forF given table. +// This function is usually used for custom interface definition, you do not need call it manually. +// The parameter `data` can be type of map/gmap/struct/*struct/[]map/[]struct, etc. +// Eg: +// Data(g.Map{"uid": 10000, "name":"john"}) +// Data(g.Slice{g.Map{"uid": 10000, "name":"john"}, g.Map{"uid": 20000, "name":"smith"}) +// +// The parameter `option` values are as follows: +// 0: insert: just insert, if there's unique/primary key in the data, it returns error; +// 1: replace: if there's unique/primary key in the data, it deletes it from table and inserts a new one; +// 2: save: if there's unique/primary key in the data, it updates it or else inserts a new one; +// 3: ignore: if there's unique/primary key in the data, it ignores the inserting; +func (d *DriverWrapperDB) DoInsert(ctx context.Context, link Link, table string, list List, option DoInsertOption) (result sql.Result, err error) { + // Convert data type before commit it to underlying db driver. + for i, item := range list { + list[i], err = d.DB.ConvertDataForRecord(ctx, item) + if err != nil { + return nil, err + } + } + return d.DB.DoInsert(ctx, link, table, list, option) +} diff --git a/database/gdb/gdb_model_insert.go b/database/gdb/gdb_model_insert.go index 2b3838a5ae4..ce95193483b 100644 --- a/database/gdb/gdb_model_insert.go +++ b/database/gdb/gdb_model_insert.go @@ -39,11 +39,7 @@ func (m *Model) Batch(batch int) *Model { // Data(g.Map{"uid": 10000, "name":"john"}) // Data(g.Slice{g.Map{"uid": 10000, "name":"john"}, g.Map{"uid": 20000, "name":"smith"}). func (m *Model) Data(data ...interface{}) *Model { - var ( - err error - ctx = m.GetCtx() - model = m.getModel() - ) + var model = m.getModel() if len(data) > 1 { if s := gconv.String(data[0]); gstr.Contains(s, "?") { model.data = s @@ -88,10 +84,7 @@ func (m *Model) Data(data ...interface{}) *Model { } list := make(List, reflectInfo.OriginValue.Len()) for i := 0; i < reflectInfo.OriginValue.Len(); i++ { - list[i], err = m.db.ConvertDataForRecord(ctx, reflectInfo.OriginValue.Index(i).Interface()) - if err != nil { - panic(err) - } + list[i] = DataToMapDeep(reflectInfo.OriginValue.Index(i).Interface()) } model.data = list @@ -108,24 +101,15 @@ func (m *Model) Data(data ...interface{}) *Model { list = make(List, len(array)) ) for i := 0; i < len(array); i++ { - list[i], err = m.db.ConvertDataForRecord(ctx, array[i]) - if err != nil { - panic(err) - } + list[i] = DataToMapDeep(array[i]) } model.data = list } else { - model.data, err = m.db.ConvertDataForRecord(ctx, data[0]) - if err != nil { - panic(err) - } + model.data = DataToMapDeep(data[0]) } case reflect.Map: - model.data, err = m.db.ConvertDataForRecord(ctx, data[0]) - if err != nil { - panic(err) - } + model.data = DataToMapDeep(data[0]) default: model.data = data[0] @@ -260,7 +244,7 @@ func (m *Model) doInsertWithOption(ctx context.Context, insertOption InsertOptio } var ( list List - now = gtime.Now().Time + now = gtime.Now() fieldNameCreate = m.getSoftFieldNameCreated("", m.tablesInit) fieldNameUpdate = m.getSoftFieldNameUpdated("", m.tablesInit) ) @@ -278,53 +262,34 @@ func (m *Model) doInsertWithOption(ctx context.Context, insertOption InsertOptio case List: list = value - for i, v := range list { - list[i], err = m.db.ConvertDataForRecord(ctx, v) - if err != nil { - return nil, err - } - } case Map: - var listItem map[string]interface{} - if listItem, err = m.db.ConvertDataForRecord(ctx, value); err != nil { - return nil, err - } - list = List{listItem} + list = List{value} default: + // It uses gconv.Map here to simply fo the type converting from interface{} to map[string]interface{}, + // as there's another DataToMapDeep in next logic to do the deep converting. reflectInfo := reflection.OriginValueAndKind(newData) switch reflectInfo.OriginKind { // If it's slice type, it then converts it to List type. case reflect.Slice, reflect.Array: list = make(List, reflectInfo.OriginValue.Len()) for i := 0; i < reflectInfo.OriginValue.Len(); i++ { - list[i], err = m.db.ConvertDataForRecord(ctx, reflectInfo.OriginValue.Index(i).Interface()) + list[i] = DataToMapDeep(reflectInfo.OriginValue.Index(i).Interface()) } case reflect.Map: - var listItem map[string]interface{} - if listItem, err = m.db.ConvertDataForRecord(ctx, value); err != nil { - return nil, err - } - list = List{listItem} + list = List{DataToMapDeep(value)} case reflect.Struct: if v, ok := value.(iInterfaces); ok { array := v.Interfaces() list = make(List, len(array)) for i := 0; i < len(array); i++ { - list[i], err = m.db.ConvertDataForRecord(ctx, array[i]) - if err != nil { - return nil, err - } + list[i] = DataToMapDeep(array[i]) } } else { - var listItem map[string]interface{} - if listItem, err = m.db.ConvertDataForRecord(ctx, value); err != nil { - return nil, err - } - list = List{listItem} + list = List{DataToMapDeep(value)} } default: diff --git a/database/gdb/gdb_model_update.go b/database/gdb/gdb_model_update.go index f819336b748..e52fbf77a16 100644 --- a/database/gdb/gdb_model_update.go +++ b/database/gdb/gdb_model_update.go @@ -9,9 +9,10 @@ package gdb import ( "database/sql" "fmt" - "github.com/gogf/gf/v2/internal/intlog" "reflect" + "github.com/gogf/gf/v2/internal/intlog" + "github.com/gogf/gf/v2/errors/gcode" "github.com/gogf/gf/v2/errors/gerror" "github.com/gogf/gf/v2/internal/reflection" @@ -57,11 +58,7 @@ func (m *Model) Update(dataAndWhere ...interface{}) (result sql.Result, err erro switch reflectInfo.OriginKind { case reflect.Map, reflect.Struct: - var dataMap map[string]interface{} - dataMap, err = m.db.ConvertDataForRecord(ctx, m.data) - if err != nil { - return nil, err - } + var dataMap = DataToMapDeep(m.data) // Automatically update the record updating time. if fieldNameUpdate != "" { dataMap[fieldNameUpdate] = gtime.Now() From 1367580709ffe58c9ca42f0ea47df4fc1cbefc18 Mon Sep 17 00:00:00 2001 From: John Guo Date: Thu, 3 Aug 2023 21:07:38 +0800 Subject: [PATCH 3/6] up --- database/gdb/gdb_func.go | 4 ++++ database/gdb/gdb_model_insert.go | 16 ++++++++-------- database/gdb/gdb_model_update.go | 2 +- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/database/gdb/gdb_func.go b/database/gdb/gdb_func.go index 90d02470e39..a5e0a0311a3 100644 --- a/database/gdb/gdb_func.go +++ b/database/gdb/gdb_func.go @@ -209,6 +209,10 @@ func GetInsertOperationByOption(option InsertOption) string { return operator } +func anyValueToMapBeforeToRecord(value interface{}) map[string]interface{} { + return gconv.Map(value, structTagPriority...) +} + // DataToMapDeep converts `value` to map type recursively(if attribute struct is embedded). // The parameter `value` should be type of *map/map/*struct/struct. // It supports embedded struct definition for struct. diff --git a/database/gdb/gdb_model_insert.go b/database/gdb/gdb_model_insert.go index ce95193483b..343f36a5e6d 100644 --- a/database/gdb/gdb_model_insert.go +++ b/database/gdb/gdb_model_insert.go @@ -84,7 +84,7 @@ func (m *Model) Data(data ...interface{}) *Model { } list := make(List, reflectInfo.OriginValue.Len()) for i := 0; i < reflectInfo.OriginValue.Len(); i++ { - list[i] = DataToMapDeep(reflectInfo.OriginValue.Index(i).Interface()) + list[i] = anyValueToMapBeforeToRecord(reflectInfo.OriginValue.Index(i).Interface()) } model.data = list @@ -101,15 +101,15 @@ func (m *Model) Data(data ...interface{}) *Model { list = make(List, len(array)) ) for i := 0; i < len(array); i++ { - list[i] = DataToMapDeep(array[i]) + list[i] = anyValueToMapBeforeToRecord(array[i]) } model.data = list } else { - model.data = DataToMapDeep(data[0]) + model.data = anyValueToMapBeforeToRecord(data[0]) } case reflect.Map: - model.data = DataToMapDeep(data[0]) + model.data = anyValueToMapBeforeToRecord(data[0]) default: model.data = data[0] @@ -275,21 +275,21 @@ func (m *Model) doInsertWithOption(ctx context.Context, insertOption InsertOptio case reflect.Slice, reflect.Array: list = make(List, reflectInfo.OriginValue.Len()) for i := 0; i < reflectInfo.OriginValue.Len(); i++ { - list[i] = DataToMapDeep(reflectInfo.OriginValue.Index(i).Interface()) + list[i] = anyValueToMapBeforeToRecord(reflectInfo.OriginValue.Index(i).Interface()) } case reflect.Map: - list = List{DataToMapDeep(value)} + list = List{anyValueToMapBeforeToRecord(value)} case reflect.Struct: if v, ok := value.(iInterfaces); ok { array := v.Interfaces() list = make(List, len(array)) for i := 0; i < len(array); i++ { - list[i] = DataToMapDeep(array[i]) + list[i] = anyValueToMapBeforeToRecord(array[i]) } } else { - list = List{DataToMapDeep(value)} + list = List{anyValueToMapBeforeToRecord(value)} } default: diff --git a/database/gdb/gdb_model_update.go b/database/gdb/gdb_model_update.go index e52fbf77a16..32c74967039 100644 --- a/database/gdb/gdb_model_update.go +++ b/database/gdb/gdb_model_update.go @@ -58,7 +58,7 @@ func (m *Model) Update(dataAndWhere ...interface{}) (result sql.Result, err erro switch reflectInfo.OriginKind { case reflect.Map, reflect.Struct: - var dataMap = DataToMapDeep(m.data) + var dataMap = anyValueToMapBeforeToRecord(m.data) // Automatically update the record updating time. if fieldNameUpdate != "" { dataMap[fieldNameUpdate] = gtime.Now() From 439a6573c59a7aa5fb077892b4552425ffafb76b Mon Sep 17 00:00:00 2001 From: x <42709773+xgd16@users.noreply.github.com> Date: Thu, 3 Aug 2023 21:26:33 +0800 Subject: [PATCH 4/6] Update clickhouse_test.go add: Assert auto-written time field value is equal to or later than beforeInsertTime --- contrib/drivers/clickhouse/clickhouse_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/contrib/drivers/clickhouse/clickhouse_test.go b/contrib/drivers/clickhouse/clickhouse_test.go index 2f57d9abdcf..3804cd71d51 100644 --- a/contrib/drivers/clickhouse/clickhouse_test.go +++ b/contrib/drivers/clickhouse/clickhouse_test.go @@ -277,11 +277,19 @@ func TestDriverClickhouse_InsertOneAutoDateTimeWrite(t *testing.T) { gtest.AssertNE(connect, nil) gtest.AssertEQ(createClickhouseTableVisits(connect), nil) defer dropClickhouseTableVisits(connect) + beforeInsertTime := time.Now() _, err = connect.Model("visits").Data(g.Map{ "duration": float64(grand.Intn(999)), "url": gconv.String(grand.Intn(999)), }).Insert() gtest.AssertNil(err) + // Query the inserted data to get the time field value + data, err := connect.Model("visits").One() + gtest.AssertNil(err) + // Get the time value from the inserted data + createdTime := data["created"].Time() + // Assert the time field value is equal to or after the beforeInsertTime + gtest.AssertGE(createdTime, beforeInsertTime) } func TestDriverClickhouse_InsertMany(t *testing.T) { From b9c80584e731701daefca9bb6fcad159d3ebec53 Mon Sep 17 00:00:00 2001 From: John Guo Date: Thu, 3 Aug 2023 22:03:59 +0800 Subject: [PATCH 5/6] up --- contrib/drivers/clickhouse/clickhouse.go | 3 +-- contrib/drivers/dm/dm.go | 33 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/contrib/drivers/clickhouse/clickhouse.go b/contrib/drivers/clickhouse/clickhouse.go index d5b4f99b6bc..96860e5bcd0 100644 --- a/contrib/drivers/clickhouse/clickhouse.go +++ b/contrib/drivers/clickhouse/clickhouse.go @@ -51,7 +51,6 @@ const ( filterTypePattern = `(?i)^UPDATE|DELETE` replaceSchemaPattern = `@(.+?)/([\w\.\-]+)+` needParsedSqlInCtx gctx.StrKey = "NeedParsedSql" - OrmTagForStruct = gtag.ORM driverName = "clickhouse" ) @@ -338,7 +337,7 @@ func (d *Driver) DoInsert( // ConvertDataForRecord converting for any data that will be inserted into table/collection as a record. func (d *Driver) ConvertDataForRecord(ctx context.Context, value interface{}) (map[string]interface{}, error) { - m := gconv.Map(value, OrmTagForStruct) + m := gconv.Map(value, gtag.ORM) // transforms a value of a particular type for k, v := range m { diff --git a/contrib/drivers/dm/dm.go b/contrib/drivers/dm/dm.go index cb1ad603eb0..46be212ea63 100644 --- a/contrib/drivers/dm/dm.go +++ b/contrib/drivers/dm/dm.go @@ -15,15 +15,19 @@ import ( "reflect" "strconv" "strings" + "time" _ "gitee.com/chunanyong/dm" + "github.com/gogf/gf/v2/database/gdb" "github.com/gogf/gf/v2/errors/gcode" "github.com/gogf/gf/v2/errors/gerror" "github.com/gogf/gf/v2/frame/g" + "github.com/gogf/gf/v2/os/gtime" "github.com/gogf/gf/v2/text/gregex" "github.com/gogf/gf/v2/text/gstr" "github.com/gogf/gf/v2/util/gconv" + "github.com/gogf/gf/v2/util/gtag" "github.com/gogf/gf/v2/util/gutil" ) @@ -169,6 +173,35 @@ func (d *Driver) TableFields( return fields, nil } +// ConvertDataForRecord converting for any data that will be inserted into table/collection as a record. +func (d *Driver) ConvertDataForRecord(ctx context.Context, value interface{}) (map[string]interface{}, error) { + m := gconv.Map(value, gtag.ORM) + + // transforms a value of a particular type + for k, v := range m { + switch itemValue := v.(type) { + // dm does not support time.Time, it so here converts it to time string that it supports. + case time.Time: + m[k] = gtime.New(itemValue).String() + // If the time is zero, it then updates it to nil, + // which will insert/update the value to database as "null". + if itemValue.IsZero() { + m[k] = nil + } + + // dm does not support time.Time, it so here converts it to time string that it supports. + case *time.Time: + m[k] = gtime.New(itemValue).String() + // If the time is zero, it then updates it to nil, + // which will insert/update the value to database as "null". + if itemValue == nil || itemValue.IsZero() { + m[k] = nil + } + } + } + return m, nil +} + // DoFilter deals with the sql string before commits it to underlying sql driver. func (d *Driver) DoFilter(ctx context.Context, link gdb.Link, sql string, args []interface{}) (newSql string, newArgs []interface{}, err error) { defer func() { From 8e9a975f71eb1b5fa93b34d71f431e1bd825ae91 Mon Sep 17 00:00:00 2001 From: x <895740313@qq.com> Date: Thu, 3 Aug 2023 22:18:59 +0800 Subject: [PATCH 6/6] Fix: ci failure caused by loss of accuracy of the queried creation time --- contrib/drivers/clickhouse/clickhouse_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/drivers/clickhouse/clickhouse_test.go b/contrib/drivers/clickhouse/clickhouse_test.go index 3804cd71d51..a4eb484870e 100644 --- a/contrib/drivers/clickhouse/clickhouse_test.go +++ b/contrib/drivers/clickhouse/clickhouse_test.go @@ -289,7 +289,7 @@ func TestDriverClickhouse_InsertOneAutoDateTimeWrite(t *testing.T) { // Get the time value from the inserted data createdTime := data["created"].Time() // Assert the time field value is equal to or after the beforeInsertTime - gtest.AssertGE(createdTime, beforeInsertTime) + gtest.AssertGE(createdTime.Unix(), beforeInsertTime.Unix()) } func TestDriverClickhouse_InsertMany(t *testing.T) {