Skip to content

Column Valuers broken since #690 #708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
randomjunk opened this issue Nov 16, 2017 · 1 comment
Closed

Column Valuers broken since #690 #708

randomjunk opened this issue Nov 16, 2017 · 1 comment
Labels
Milestone

Comments

@randomjunk
Copy link
Contributor

Issue description

Implement a struct with Valuer and use it as a named value in SQL statement.
We expect it to convert to driver.Value and work.
Our current code does this all over the place (using gorm).

The problem is that the recent change to named value handling (#690) returns a hard error if it can't convert the type, whereas it should return driver.ErrSkip to allow the default converter to handle it.

Example code

type Thing struct {
 content string
}

func (t Thing) Value() (driver.Value, error) {
  return t
}

Error log

sql: converting argument $5 type: unsupported type user.VerifyNotes, a struct
@randomjunk randomjunk changed the title Column converters broken since #690 Column Valuers broken since #690 Nov 16, 2017
@randomjunk
Copy link
Contributor Author

randomjunk commented Nov 16, 2017

type testValuer struct {
	value string
}

func (tv testValuer) Value() (driver.Value, error) {
	return tv.value, nil
}

func TestValuer(t *testing.T) {
	runTests(t, dsn, func(dbt *DBTest) {
		in := testValuer{"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")
		if rows.Next() {
			rows.Scan(&out)
			if in.value != out {
				dbt.Errorf("Valuer: %v != %s", in, out)
			}
		} else {
			dbt.Errorf("Valuer: no data")
		}

		dbt.mustExec("DROP TABLE IF EXISTS test")
	})
}

randomjunk added a commit to randomjunk/mysql that referenced this issue Nov 16, 2017
…rnally

Added a test that Valuer types are handled correctly. This test fails without the fix.

CheckNamedValue code was included recently and breaks existing applications:
go-sql-driver#690

Reported in:
go-sql-driver#708
randomjunk added a commit to randomjunk/mysql that referenced this issue Nov 16, 2017
…rnally

Added a test that Valuer types are handled correctly. This test fails without the fix.

CheckNamedValue code was included recently and breaks existing applications:
go-sql-driver#690

Reported in:
go-sql-driver#708
@julienschmidt julienschmidt added this to the v1.4 milestone Nov 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants