Fix encode driver.Valuer on pointer #2019
Conversation
pgx v5 introduced nil normalization for typed nils. This means that
[]byte(nil) is normalized to nil at the edge of the encoding system.
This simplified encoding logic as nil could be encoded as NULL and type
specific handling was unneeded.
However, database/sql compatibility requires Value to be called on a
nil pointer that implements driver.Valuer. This was broken by
normalizing to nil.
This commit changes the normalization logic to not normalize pointers
that directly implement driver.Valuer to nil. It still normalizes
pointers that implement driver.Valuer through implicit derefence.
e.g.
type T struct{}
func (t *T) Value() (driver.Value, error) {
return nil, nil
}
type S struct{}
func (s S) Value() (driver.Value, error) {
return nil, nil
}
(*T)(nil) will not be normalized to nil but (*S)(nil) will be.
#1566
internal/anynil/anynil.go
Outdated
| return false | ||
| } | ||
|
|
||
| if kind == reflect.Ptr { |
There was a problem hiding this comment.
For my use-case, the restriction to just pointers won't work since the type I'm using is an alias to a slice. See #1860 for some example code.
Allowing reflect.Slice here would fix my use-case, but it would probably make sense to allow for others as well.
There was a problem hiding this comment.
Good point. It now extends the same logic to all nilable values.
This extends the behavior change a little further than I had anticipated. But it is a fix for database/sql compatibility and I don't anticipate the change breaking any real world code.
Future commit will be using bytes.Clone which was implemented in Go 1.20. Also update README.md to reflect that minimum supported Go version is 1.21. But only requiring Go 1.20 in go.mod to avoid needlessly breaking old Go when it still works.
Extract logic for finding OID and converting argument to encodable value. This is in preparation for a future change for better supporting nil driver.Valuer values.
anynil.Is was already being called in all paths that anynil.NormalizeSlice was used.
The new logic checks for any type of nil at the beginning of Encode and then either treats it as NULL or calls the driver.Valuer method if appropriate. This should preserve the existing nil normalization while restoring the ability to encode nil driver.Valuer values.
|
I just refactored the whole typed nil system to exist entirely in If my design and implementation is correct, this approach should restore full driver.Valuer support while still not leaking any typed nils into the Codec system. |
|
I like the refactor. Tracing how nils were handled prior to the change was a bit complicated and this definitely simplifies it. |
pgx v5 introduced nil normalization for typed nils. This means that []byte(nil) is normalized to nil at the edge of the encoding system. This simplified encoding logic as nil could be encoded as NULL and type specific handling was unneeded.
However, database/sql compatibility requires Value to be called on a nil pointer that implements driver.Valuer. This was broken by normalizing to nil.
This commit changes the normalization logic to not normalize pointers that directly implement driver.Valuer to nil. It still normalizes pointers that implement driver.Valuer through implicit derefence.
e.g.
(*T)(nil)will not be normalized to nil but(*S)(nil)will be.#1566