jmoiron / sqlx

general purpose extensions to golang's database/sql
http://jmoiron.github.io/sqlx/
MIT License
16.3k stars 1.09k forks source link

Update reflectx to allow for optional nested structs #900

Open geeeeeeeeek opened 1 year ago

geeeeeeeeek commented 1 year ago

Based on https://github.com/jmoiron/sqlx/pull/847, but fixed an issue that optional pointer fields are not handled correctly.

Nested structs are now only instantiated when one of the database columns in that nested struct is not nil. This allows objects scanned in left/outer joins to keep their natural types (instead of setting everything to NullableX).

Example:

select house.id, owner.*,
from house
left join owner on owner.id = house.owner

Before

type House struct {
 ID      int
 Owner   *Person // if left join gives nulls, Owner is set
}

type Owner struct {
 ID sql.NullInt64 // make nullable columns even tho if table doesn't have those columns nullable
}

After

type House struct {
 ID      int
 Owner   *Person // if left join gives nulls, Owner will be nil
}

type Owner struct {
 ID int    // no need to set this to sql.NullInt
}
dlsniper commented 9 months ago

Hello, @ardan-bkennedy, and I recently stepped in to help maintain this project. We are sorting the opened issues and pull requests and would like to know if you still NEED this merged. Thank you for your contribution.

taleeus commented 8 months ago

This would be insanely useful 🙏🏻

AlexandrePhilibert commented 5 months ago

Would love for this to be merged. I have been using sqlx with these changes for some time as it is a feature I find really useful.

kszafran commented 1 month ago

@dlsniper This issue is precisely what forced me to ditch sqlx many years ago. Now I ran into it again in another project... Hoping to see this merged soon 🤞🏻

kszafran commented 1 month ago

I tried this change in my project, and there are definitely breaking changes. The code copies from stdlib convertAssign, but it's not fully compatible with it. For example, without this change scanning an int64 value into string field works fine, but with this change it fails. I'm looking at the code trying to figure out I could address this issue.

kszafran commented 1 month ago

@dlsniper @ardan-bkennedy I created another followup PR: https://github.com/jmoiron/sqlx/pull/950. It addresses incompatibility issues, adds tests, and is much shorter. It would be great if you could take a look at it!