Skip to content
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

Update reflectx to allow for optional nested structs #900

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

geeeeeeeeek
Copy link

@geeeeeeeeek geeeeeeeeek commented Nov 1, 2023

Based on #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
}

@geeeeeeeeek geeeeeeeeek marked this pull request as ready for review November 1, 2023 19:51
@dlsniper dlsniper added the requires attention The PR looks like it could potentially break things. Definitely requires more testing label Feb 1, 2024
@dlsniper
Copy link
Collaborator

dlsniper commented Feb 1, 2024

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
Copy link

taleeus commented Mar 11, 2024

This would be insanely useful 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
requires attention The PR looks like it could potentially break things. Definitely requires more testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants