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

fix: has many relation with driver.Valuer #1080

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

thecampagnards
Copy link
Contributor

@thecampagnards thecampagnards commented Nov 27, 2024

Hello,
I have some trouble to use driver.Valuer on has many.
Here a pr, tell me if it's the wrong place
Thx !

@thecampagnards thecampagnards changed the title fix: m2m relation with driver.Valuer fix: has many relation with driver.Valuer Nov 27, 2024
@Tiscs
Copy link
Collaborator

Tiscs commented Nov 28, 2024

LGTM.

Thanks for your contribution.

Based on the unit tests you provided, I ran some tests. Before the indirectFieldValue was modified, the Key value type in the User model's baseValues was inconsistent with the Deck model's structKey value type, causing parkStruct to return an error when matching.

I also tested the case where User.DeskID was a sql.NullInt64{Valid: false} or defined as *int64, and the code ran as expected.

I think this fix is ​​correct, @vmihailenco what do you think?

@Tiscs Tiscs self-assigned this Nov 28, 2024
@thecampagnards
Copy link
Contributor Author

LGTM.

Thanks for your contribution.

Based on the unit tests you provided, I ran some tests. Before the indirectFieldValue was modified, the Key value type in the User model's baseValues was inconsistent with the Deck model's structKey value type, causing parkStruct to return an error when matching.

I also tested the case where User.DeskID was a sql.NullInt64{Valid: false} or defined as *int64, and the code ran as expected.

I think this fix is ​​correct, @vmihailenco what do you think?

Thx ! Do you want me to add a Valid: false test ?
Thank you for checking the PR !

@j2gg0s j2gg0s merged commit 8b05a13 into uptrace:master Nov 29, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants