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 QueryRowsPartial getTaggedFieldValueMap func #2884

Merged
merged 8 commits into from
Mar 11, 2023

Conversation

fulldog
Copy link
Contributor

@fulldog fulldog commented Feb 15, 2023

When using orm, a problem was found. When you use QueryRowsPartial to get a small number of query fields, the scanner scans the struct in the order it was passed in, causing an err
The following structure:
image
image
err:
image

@fulldog fulldog changed the title reload getTaggedFieldValueMap fix QueryRowsPartial getTaggedFieldValueMap func Feb 15, 2023
@fulldog
Copy link
Contributor Author

fulldog commented Feb 16, 2023

@kevin0527 大佬看一下问题

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@kevin0527 big guy look at the problem

@kevwan kevwan force-pushed the fix-QueryRowsPartial branch from d5e0374 to 24fe846 Compare February 22, 2023 06:12
@kevwan kevwan self-requested a review February 22, 2023 06:13
@kevwan
Copy link
Contributor

kevwan commented Feb 22, 2023

Please check and fix UT.

@fulldog
Copy link
Contributor Author

fulldog commented Feb 22, 2023

@kevwan The bug has been fixed and tested

@kevwan
Copy link
Contributor

kevwan commented Feb 26, 2023

Thanks for your PR! Please add unit tests.

@fulldog
Copy link
Contributor Author

fulldog commented Feb 27, 2023

@kevwan sorry for this.
test case is in core/stores/sqlx/orm_test.go,func TestAnonymousStructPr().
that case has been tested in orm_test.go

@codecov
Copy link

codecov bot commented Feb 27, 2023

Codecov Report

Merging #2884 (da986d7) into master (f77e2c9) will decrease coverage by 0.27%.
The diff coverage is 100.00%.

❗ Current head da986d7 differs from pull request most recent head c45ea0c. Consider uploading reports for the commit c45ea0c to get more accurate results

Impacted Files Coverage Δ
core/stores/sqlx/orm.go 72.52% <100.00%> (+6.43%) ⬆️

... and 29 files with indirect coverage changes

@fulldog fulldog force-pushed the fix-QueryRowsPartial branch from 0c856dc to 74b73d8 Compare March 2, 2023 08:41
@kevwan kevwan force-pushed the fix-QueryRowsPartial branch from da986d7 to ae8f61f Compare March 4, 2023 11:50
@kevwan kevwan requested a review from re-dylan March 4, 2023 11:56
@kevwan kevwan added this to the v1.5.1 milestone Mar 4, 2023
yongkun.xiong added 8 commits March 11, 2023 15:18

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
fix bug

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
add test case

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@kevwan kevwan force-pushed the fix-QueryRowsPartial branch from ae8f61f to c45ea0c Compare March 11, 2023 07:18
@kevwan kevwan added this pull request to the merge queue Mar 11, 2023
Merged via the queue into zeromicro:master with commit e735915 Mar 11, 2023
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.

None yet

3 participants