Update reflectx to allow for optional nested structs
Based on #847 and #900, but simplified and fully compatible with the standard library scanning behavior.
Compared to previous PRs:
- Instead of halfway copying
convertAssignfrom the standard library (which was done in a way that introduced incompatibilities), I used//go:linknameto call the original function directly. Normally, this isn't recommended. However, Go authors recognize that some libraries do that, and marked selected functions to avoid breaking changes, includingconvertAssign. See https://go.dev/issue/67401. - I simplified the code by replacing the mutable
ObjectContextwith a basicfunctype. - I added tests for an optional struct inside an optional struct.
All in all, the changes are now much more succinct, which I hope will help get this PR reviewed and merged.
Note that this is a minor breaking change. If someone scans into a struct that has a struct pointer field and all the fields in the nested struct can accept NULL values, this would change the observed behavior. For example:
type Inner struct {
A *string
B *int64
}
type Outer struct {
Inner *Inner
}
Previously, when NULL was scanned into the fields of the nested struct, the nested struct would still be present (outer.Inner would be &Inner{}). Now it'll be nil.
Moreover, if someone has custom Scanner implementations that handle nil in some non-standard way (e.g. by initializing the value somehow), this will affect them. Such scanners won't be called on fields inside nested structs (including non-optional nested structs!) until the first non-NULL value is encountered. So, for example:
type JSON json.RawMessage
func (j *JSON) Scan(src any) error {
if src == nil {
*j = "{}"
return
}
...
}
type Outer struct {
Inner struct {
Stuff JSON
Thing string
}
}
Scan on Stuff wouldn't be called with a NULL value. But if we reorder fields:
Thing string
Stuff JSON
then it would be called (assuming we're scanning non-NULL into Thing)...
While there are different ways we could go about it, I'm thinking the safest approach would be to make this feature opt-in, e.g. with an omitempty attribute:
type Outer struct {
Inner *Inner `db:inner,omitempty`
}
If there's no omitempty attribute, we'd keep the old behavior. That way this change would be fully backwards compatible, plus adding an explicit omitempty would make the intended behavior clear. It might complicate the implementation a little bit (especially for deeply nested structs), but I think it would make this change overall safer.
Let me know what you think, and I could update this PR.
BTW, note that this feature makes it possible to have optional nested structs without the use of pointers, too. For example:
type Inner struct {
A string
B int64
}
type Outer struct {
Inner Inner
}
Normally, if I used a LEFT JOIN and a and b would be NULL, scanning would fail here, because NULL cannot be decoded into string or int64. However, with this feature (and the omitempty attribute), scanning would succeed, simply leaving the Inner struct empty. From that point of view, the support for optional nested structs is even more flexible, and this would be another argument to make the behavior configurable with omitempty. We'd just need to document well what omitempty actually does.