From 232eaa8bc15b2e34ef0a28f2fe6ad88bd7a981f2 Mon Sep 17 00:00:00 2001 From: Linh Tran Tuan Date: Fri, 17 Nov 2017 09:15:04 +0700 Subject: [PATCH 1/5] Fix valuer converter with right to return error Valuer has the right to return error instead of driver.ErrSkip --- AUTHORS | 2 ++ connection_go18.go | 6 +----- driver_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ statement.go | 6 ++++++ 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/AUTHORS b/AUTHORS index 1ace1fa45..37e6fc940 100644 --- a/AUTHORS +++ b/AUTHORS @@ -46,6 +46,7 @@ Kamil Dziedzic Kevin Malachowski Lennart Rudolph Leonardo YongUk Kim +Linh Tran Tuan Lion Yang Luca Looz Lucas Liu @@ -72,6 +73,7 @@ Zhenye Xie Barracuda Networks, Inc. Counting Ltd. +CIS Viet Nam JSC Google Inc. Keybase Inc. Pivotal Inc. diff --git a/connection_go18.go b/connection_go18.go index 65cc63ef2..1306b70b7 100644 --- a/connection_go18.go +++ b/connection_go18.go @@ -197,10 +197,6 @@ func (mc *mysqlConn) startWatcher() { } func (mc *mysqlConn) CheckNamedValue(nv *driver.NamedValue) (err error) { - value, err := converter{}.ConvertValue(nv.Value) - if err != nil { - return driver.ErrSkip - } - nv.Value = value + nv.Value, err = converter{}.ConvertValue(nv.Value) return } diff --git a/driver_test.go b/driver_test.go index 392e752a3..81410d99a 100644 --- a/driver_test.go +++ b/driver_test.go @@ -529,6 +529,47 @@ func TestValuer(t *testing.T) { }) } +type testValuerWithValidation struct { + value string +} + +func (tv testValuerWithValidation) Value() (driver.Value, error) { + if len(tv.value) == 0 { + return nil, fmt.Errorf("Invalid string valuer. Value must not be empty") + } + + return tv.value, nil +} + +func TestValuerWithValidation(t *testing.T) { + runTests(t, dsn, func(dbt *DBTest) { + in := testValuerWithValidation{"a_value"} + var out string + var rows *sql.Rows + + dbt.mustExec("CREATE TABLE test (value VARCHAR(255)) CHARACTER SET utf8") + dbt.mustExec("INSERT INTO test VALUES (?)", in) + + rows = dbt.mustQuery("SELECT value FROM test") + defer rows.Close() + + if rows.Next() { + rows.Scan(&out) + if in.value != out { + dbt.Errorf("Valuer: %v != %s", in, out) + } + } else { + dbt.Errorf("Valuer: no data") + } + + if _, err := dbt.db.Exec("INSERT INTO test VALUES (?)", testValuerWithValidation{""}); err == nil { + dbt.Errorf("Failed to check valuer error") + } + + dbt.mustExec("DROP TABLE IF EXISTS test") + }) +} + type timeTests struct { dbtype string tlayout string diff --git a/statement.go b/statement.go index 4870a307c..98e57bcd8 100644 --- a/statement.go +++ b/statement.go @@ -137,6 +137,12 @@ func (c converter) ConvertValue(v interface{}) (driver.Value, error) { return v, nil } + if v != nil { + if valuer, ok := v.(driver.Valuer); ok { + return valuer.Value() + } + } + rv := reflect.ValueOf(v) switch rv.Kind() { case reflect.Ptr: From 1dcddc811dae9956b85ba7bd9bfdcdc146b92f95 Mon Sep 17 00:00:00 2001 From: Linh Tran Tuan Date: Fri, 17 Nov 2017 09:24:35 +0700 Subject: [PATCH 2/5] Fix valuer converter Valuer has the right to return error instead of returning driver.ErrSkip as current --- AUTHORS | 1 - 1 file changed, 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index 37e6fc940..c648e1835 100644 --- a/AUTHORS +++ b/AUTHORS @@ -73,7 +73,6 @@ Zhenye Xie Barracuda Networks, Inc. Counting Ltd. -CIS Viet Nam JSC Google Inc. Keybase Inc. Pivotal Inc. From 627775def89d1a817949df652ec26d9a67c45191 Mon Sep 17 00:00:00 2001 From: Linh Tran Tuan Date: Fri, 17 Nov 2017 09:49:45 +0700 Subject: [PATCH 3/5] Update test case --- driver_test.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/driver_test.go b/driver_test.go index 81410d99a..830b6e70a 100644 --- a/driver_test.go +++ b/driver_test.go @@ -547,10 +547,10 @@ func TestValuerWithValidation(t *testing.T) { var out string var rows *sql.Rows - dbt.mustExec("CREATE TABLE test (value VARCHAR(255)) CHARACTER SET utf8") - dbt.mustExec("INSERT INTO test VALUES (?)", in) + dbt.mustExec("CREATE TABLE testValuer (value VARCHAR(255)) CHARACTER SET utf8") + dbt.mustExec("INSERT INTO testValuer VALUES (?)", in) - rows = dbt.mustQuery("SELECT value FROM test") + rows = dbt.mustQuery("SELECT value FROM testValuer") defer rows.Close() if rows.Next() { @@ -562,11 +562,19 @@ func TestValuerWithValidation(t *testing.T) { dbt.Errorf("Valuer: no data") } - if _, err := dbt.db.Exec("INSERT INTO test VALUES (?)", testValuerWithValidation{""}); err == nil { + if _, err := dbt.db.Exec("INSERT INTO testValuer VALUES (?)", testValuerWithValidation{""}); err == nil { dbt.Errorf("Failed to check valuer error") } - dbt.mustExec("DROP TABLE IF EXISTS test") + if _, err := dbt.db.Exec("INSERT INTO testValuer VALUES (?)", nil); err == nil { + dbt.Errorf("Failed to check nil") + } + + if _, err := dbt.db.Exec("INSERT INTO testValuer VALUES (?)", map[string]bool{}); err == nil { + dbt.Errorf("Failed to check not valuer") + } + + dbt.mustExec("DROP TABLE IF EXISTS testValuer") }) } From cf66739eaeb01ae732304bb52266d778c1d55957 Mon Sep 17 00:00:00 2001 From: Linh Tran Tuan Date: Fri, 17 Nov 2017 09:54:01 +0700 Subject: [PATCH 4/5] Revert "Update test case" This reverts commit 627775def89d1a817949df652ec26d9a67c45191. --- driver_test.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/driver_test.go b/driver_test.go index 830b6e70a..81410d99a 100644 --- a/driver_test.go +++ b/driver_test.go @@ -547,10 +547,10 @@ func TestValuerWithValidation(t *testing.T) { var out string var rows *sql.Rows - dbt.mustExec("CREATE TABLE testValuer (value VARCHAR(255)) CHARACTER SET utf8") - dbt.mustExec("INSERT INTO testValuer VALUES (?)", in) + dbt.mustExec("CREATE TABLE test (value VARCHAR(255)) CHARACTER SET utf8") + dbt.mustExec("INSERT INTO test VALUES (?)", in) - rows = dbt.mustQuery("SELECT value FROM testValuer") + rows = dbt.mustQuery("SELECT value FROM test") defer rows.Close() if rows.Next() { @@ -562,19 +562,11 @@ func TestValuerWithValidation(t *testing.T) { dbt.Errorf("Valuer: no data") } - if _, err := dbt.db.Exec("INSERT INTO testValuer VALUES (?)", testValuerWithValidation{""}); err == nil { + if _, err := dbt.db.Exec("INSERT INTO test VALUES (?)", testValuerWithValidation{""}); err == nil { dbt.Errorf("Failed to check valuer error") } - if _, err := dbt.db.Exec("INSERT INTO testValuer VALUES (?)", nil); err == nil { - dbt.Errorf("Failed to check nil") - } - - if _, err := dbt.db.Exec("INSERT INTO testValuer VALUES (?)", map[string]bool{}); err == nil { - dbt.Errorf("Failed to check not valuer") - } - - dbt.mustExec("DROP TABLE IF EXISTS testValuer") + dbt.mustExec("DROP TABLE IF EXISTS test") }) } From 4eb2c8d2a751795bb4e7af077bb6421e7ca9688c Mon Sep 17 00:00:00 2001 From: Linh Tran Tuan Date: Fri, 17 Nov 2017 09:54:53 +0700 Subject: [PATCH 5/5] Fix test case --- driver_test.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/driver_test.go b/driver_test.go index 81410d99a..224a24c53 100644 --- a/driver_test.go +++ b/driver_test.go @@ -547,10 +547,10 @@ func TestValuerWithValidation(t *testing.T) { var out string var rows *sql.Rows - dbt.mustExec("CREATE TABLE test (value VARCHAR(255)) CHARACTER SET utf8") - dbt.mustExec("INSERT INTO test VALUES (?)", in) + dbt.mustExec("CREATE TABLE testValuer (value VARCHAR(255)) CHARACTER SET utf8") + dbt.mustExec("INSERT INTO testValuer VALUES (?)", in) - rows = dbt.mustQuery("SELECT value FROM test") + rows = dbt.mustQuery("SELECT value FROM testValuer") defer rows.Close() if rows.Next() { @@ -562,11 +562,19 @@ func TestValuerWithValidation(t *testing.T) { dbt.Errorf("Valuer: no data") } - if _, err := dbt.db.Exec("INSERT INTO test VALUES (?)", testValuerWithValidation{""}); err == nil { + if _, err := dbt.db.Exec("INSERT INTO testValuer VALUES (?)", testValuerWithValidation{""}); err == nil { dbt.Errorf("Failed to check valuer error") } - dbt.mustExec("DROP TABLE IF EXISTS test") + if _, err := dbt.db.Exec("INSERT INTO testValuer VALUES (?)", nil); err != nil { + dbt.Errorf("Failed to check nil") + } + + if _, err := dbt.db.Exec("INSERT INTO testValuer VALUES (?)", map[string]bool{}); err == nil { + dbt.Errorf("Failed to check not valuer") + } + + dbt.mustExec("DROP TABLE IF EXISTS testValuer") }) }