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

Add test case for case sensitive table with two columns that have the same name but different casing #408

Closed
lucyzhang929 opened this issue Oct 19, 2022 · 6 comments · Fixed by #619

Comments

@lucyzhang929
Copy link
Contributor

No description provided.

@lucyzhang929
Copy link
Contributor Author

Closing this issue - I don't think we can handle two columns with the same name but different casing since the POCO needs to have unique member names.

@Charles-Gagnon
Copy link
Contributor

Charles-Gagnon commented Jan 5, 2023

When you say "needs to have unique members names" do you mean by convention of what we require? Because you can have multiple members that differ only in case in C#

https://dotnetfiddle.net/GMRPlO

And now that we require the casing to be the same as in the DB this should be a valid scenario for us to support - or am I missing something that would make this not possible?

@lucyzhang929
Copy link
Contributor Author

Ah I see, I mistook the CA1708 convention rule for an actual error. In that case this should be a valid scenario.

@lucyzhang929 lucyzhang929 reopened this Jan 5, 2023
@lucyzhang929
Copy link
Contributor Author

One issue I ran into while working on an input binding for this scenario is with Newtonsoft.Json deserialization being case insensitive. When the POCO contains "Name" and "NAME", Newtonsoft.Json throws this error Newtonsoft.Json: A member with the name 'name' already exists on 'Microsoft.Azure.WebJobs.Extensions.Sql.Samples.Common.ProductWithTwoNames'. Use the JsonPropertyAttribute to specify another name.

We would have to switch over to using System.Text.Json since Newtonsoft.Json doesn't support case sensitive deserialization. JamesNK/Newtonsoft.Json#815

@Charles-Gagnon
Copy link
Contributor

Ah, yeah ok. The JSON serialization is something that the AF folks have suggested we use from their configuration instead anyways so if we did switch anything it would just be to do everything there (which means it could still potentially have this problem).

I doubt this is going to be a common use case though so doesn't seem like something we need to spend a bunch of time fixing - rather for now I think just documenting that we don't allow column names that only differ in casing.

@dzsquared @chlafreniere That sound reasonable to you?

@chlafreniere
Copy link
Contributor

Seems reasonable to me

lucyzhang929 added a commit that referenced this issue Jan 9, 2023
Fixes #408. Documenting that the table cannot contain two columns that only differ by case.
lucyzhang929 added a commit that referenced this issue Jan 10, 2023
* Update README 

Fixes #408. Documenting that the table cannot contain two columns that only differ by case.

* Update README.md

Co-authored-by: Charles Gagnon <[email protected]>

Co-authored-by: Charles Gagnon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants