efcore
efcore copied to clipboard
When creating the "missing column" error message, use the same (case insensitive) column name comparer used in field value retrieval
Issue #33748
This addresses an issue where a column with different casing to the entity's field would be selected as the "first missing column" when assembling the error message. The field name lookup's key comparer is case insensitive, and using it here ensures that all column name comparisons will use the same comparer.
- [x] I've read the guidelines for contributing and seen the walkthrough
- [ ] I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
- [x] The code builds and tests pass locally (also verified by our automated build checks)
- [x] Commit messages follow this format:
Summary of the changes
- Detail 1
- Detail 2
Fixes #bugnumber
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] Code follows the same patterns and style as existing code in this repo
@dotnet-policy-service agree
On Sat, 18 May 2024, 12:46 pm dotnet-policy-service[bot], < @.***> wrote:
@simonmckenzie https://github.com/simonmckenzie the command you issued was incorrect. Please try again.
Examples are:
@dotnet-policy-service agree
and
@dotnet-policy-service agree company="your company"
— Reply to this email directly, view it on GitHub https://github.com/dotnet/efcore/pull/33749#issuecomment-2118609186, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEGD3MWBKEUT4SBD65KGC7DZC26GTAVCNFSM6AAAAABH5AZK5GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJYGYYDSMJYGY . You are receiving this because you were mentioned.Message ID: @.***>
Note: this PR is waiting on a design discussion on #33748
Hi @roji,
Since the parent issue (#33748) has been put into the backlog pending work on provider-level case-sensitivity settings, this PR is now in an uncertain state.
I would argue that since it addresses a known bug (inconsistent case comparison within the BufferedDataReader), it makes sense to still proceed with this fix now. Additionally, its centralisation of the column comparison logic (plus extra tests) should help with the future case-sensitivity work.
If this isn't desired, I guess I should close this PR.
Please tell me what you think.
Sorry, I forgot a PR was out - should have commented here. Given that we don't intend to break e.g. the SQL Server provider by changing to be case-sensitive, I think fixing the bug definitely still makes sense - one of us will review it soon.
Thanks.