Repodb: Bug: Property mapping fails with Turkish locale

[mertkokusen]

Thanks for this library, it really has everything I was missing with other micro ORMs.

Property names that contain uppercase letter "I" causes property mapping to fail on some machines because of culture (ex. on a machine with turkish windows). using ToLowerInvariant instead of ToLower in resolvers solves this issue.

[mikependon]

Thanks @mertkokusen. I would assume your stacktrace is different from the root cause you mentioned. Can you paste the exception here? Or, a very small steps to replicate it would be very great. Thanks!

[mertkokusen]

Unfortunately, there is no exception, just the properties that contain letter I are not filled with values from the DB.

I cloned RepoDB and ran tests with RepoDb.Sqlite solution (in in-memory mode). Initially QueryTests were failing I replaced all ToLower calls with ToLowerInvariant and all tests in QueryTests are passing. I have experienced this problem with other libraries in the last couple of years because in Turkish uppercase i is İ and lowercase I is ı, which causes lots of headaches.

Running tests on a machine with windows installed in turkish should reproduce the issue.

[mikependon]

@mertkokusen - thank you for the clarity. Will do cover this on my next release.

[mertkokusen]

@mikependon thanks

[mikependon]

@mertkokusen - would you be able to issue a PR in relation to this? It is much better actually, specially you already cloned it to your machine and had made the tests passed there. I will do cover all the other needed tests. Thanks in advance! Please do let me know if you can make it.

[mertkokusen]

@mikependon sure I will make a PR :)

[mertkokusen]

@mikependon made the PR

[mikependon]

Big Thanks. I am in a middle of refactoring the core compiler of RepoDb. I will handle the case of FunctionFactory.cs, would it be possible to exclude it from your PR? Lastly, can you also handle the string.Equals()?

[mertkokusen]

@mikependon Sure I will exclude FunctionFactory.cs (the one in RepoDb.Core solution). But excluding this file results in some tests failing. Is it ok like this?
About string.Equals(), all calls to it uses StringComparison.OrdinalIgnoreCase and I think it should be fine this way but if you have something else in mind I would like to help

[mikependon]

Thanks. I know that the StringComparisson.OrdinalIgnoreCase is the fastest, but was not it to address the locale problem, you should impose the StringComparisson.InvariantCultureIgnoreCase? It should not be a performance problem at all, we only use the comparissons before the actual compilation.

For the failing tests, I guess it is OK. It will only fail in your side due to locale, but once I committed the changes in the FunctionFactory class, then everything will work as you expect.

Hope this helps.

[mertkokusen]

StringComparisson.InvariantCultureIgnoreCasemakes additional linguistic decisions like character expantions (ex. ss being considered equal to ß). I think in our situation the way to go is to keep using OrdinalIgnoreCase which makes a byte by byte comparison.

I took the info from here
String comparison best practices

[mikependon]

That is correct, that makes the string comparisson slower. But would not it unresolving the special chars automatically? In anyway, for as long your PR is working your local machine, then that is enough. I will make sure to handle the core compiler changes. Thanks for the PR

[mikependon]

The fix is now available at RepoDb v1.12.0-beta4 and RepoDb.SqlServer v1.1.0-beta2.

> Install-Package RepoDb -version 1.12.0-beta4
> Install-Package RepoDb.SqlServer -version 1.1.0-beta2

We are closing this ticket now. Kindly let us know if you still have questions.