Conversation
The number grammar also accepted the decimal separator of the current culture. Under de-DE, Rating > 4,5 parsed, and the value conversion read it with the invariant culture as 45. The API returned wrong rows with no error. The number grammar, the count operator value, and the literal checks in IsPropertyPath now use the invariant culture only. A number always uses the '.' decimal point. BREAKING CHANGE: a number with a decimal comma, for example Rating > 4,5, throws a QueryKitException in every culture. Before, a server with a comma culture read this value as 45. Send the number with a '.' decimal point, for example Rating > 4.5.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR makes the number grammar use the invariant culture only, so a decimal comma is not accepted. This is a breaking change. It is one of the six breaking changes that were removed from #114 so that main stays compatible with v1.14.2.
Status: for later consideration. Do not merge this PR now. The captain decides on each breaking change separately. If this change is accepted, it needs a major version.
Main already parses
4.5in every culture, and list numbers always use.. This PR adds only the strict invariant grammar that rejects4,5.Old behavior (v1.14.2 and main)
CurrentCulture. Under de-DE,Rating > 4.5fails to parse.Rating > 4,5parses, and the value conversion reads it with the invariant culture as45.Rating > 4.5parses in every culture. Under de-DE,Rating > 4,5still gives45.New behavior
The number grammar uses the invariant culture only.
Rating > 4.5means 4.5 in every culture.Rating > 4,5throws aQueryKitExceptionin every culture.The change removes
UnsignedNumberParserandListNumberParser. The number grammar, the list grammar, the count operator value, and the literal checks inIsPropertyPathuse the invariant culture only.Example
Proof from the
verify-querykitharness, with the culture set to de-DE. Each row gives the same result in memory and on Postgres.Price > 4,545)ParsingException: "unexpected ','; expected end of input (Line 1, Column 10)"Price > 4.5Justification
The v1.14.2 result for
4,5is45, which is not the value that any user means. This result is silent, so the API returns wrong rows with no error. A filter string is a machine format, and the same filter must give the same result on every server.The strict invariant grammar gives one meaning for
4.5everywhere and rejects4,5with a clear error. The break affects only a consumer on a comma culture that sends4,5and depends on the value45.Migration
Send the number with a
.decimal point, for exampleRating > 4.5.Tests
decimal_value_with_decimal_comma_is_not_accepted, for de-DE, fr-FR, and en-US. It expectsQueryKitException.dotnet test: 346 unit tests and 268 integration tests pass.