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

Added offset when scanning the result back to struct #5143

Closed
wants to merge 3 commits into from
Closed

Added offset when scanning the result back to struct #5143

wants to merge 3 commits into from

Conversation

Joker666
Copy link
Contributor

@Joker666 Joker666 commented Mar 8, 2022

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

The solution is inspired by Gorm V1. To avoid always picking the same value from the LookUpField method we create a fields array and keep track of already picked values using an offset. In Gorm 1, this was the default way to scan results back.

close #5142

This fixes the demo PR for the playground - go-gorm/playground#443

User Case Description

The issue in question occurs because we are using the LookUpField method of the schema package. This method just looks for the name in the map and returns the first field it gets. So we get this scenario where two tables have the same column names and we do a JOIN query, it returns values that might be from a different table to a struct that should not have it.

@Joker666
Copy link
Contributor Author

@huacnlee please take a look

@jinzhu
Copy link
Member

jinzhu commented Mar 12, 2022

Hello @Joker666

Can you add some tests?

@Joker666
Copy link
Contributor Author

@jinzhu Added a test. But I am not sure why this check has failed though.

https://github.com/go-gorm/gorm/runs/5538082130?check_suite_focus=true

Haven't worked on anything for the Coupon model. And shows permission issue. Weird

@Joker666
Copy link
Contributor Author

I guess that was a random failure

@jinzhu jinzhu closed this in f3e2da5 Mar 17, 2022
@jinzhu
Copy link
Member

jinzhu commented Mar 17, 2022

Hi @Joker666

This PR has already been merged in this commit. f3e2da5

Thank you for your pull request.

@Joker666
Copy link
Contributor Author

Hi, @jinzhu I missed that. Thanks for merging. I just deleted my comment once I found out that it got merged and you commented.

So, I'm closing the issue as well.

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.

Gorm V2 cannot handle JOIN for tables with the same column name
2 participants