fix: fixed lower case for "IN" operator in queryFilters [DHIS2-11770] #8724
Conversation
This file contains 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
gnespolino
requested review from
larshelge,
stian-sandvold,
d-bernat and
maikelarabori
maikelarabori
approved these changes
Sep 14, 2021
|
Kudos, SonarCloud Quality Gate passed! |
larshelge
reviewed
Sep 15, 2021
| @@ -63,13 +65,16 @@ public InQueryFilter( String field, QueryFilter queryFilter ) | |||
| * @param encodedFilter actual "in" parameters | |||
| * @return a SQL condition representing this InQueryFilter | |||
| */ | |||
| public String getSqlFilter( String encodedFilter ) | |||
| public String getSqlFilter( String encodedFilter, boolean isText ) | |||
There was a problem hiding this comment.
Please add a description to explain the initial problem and why this fixes it :-) My understanding is we already quote event analytics query filters but I might be wrong.
stian-sandvold
approved these changes
Sep 15, 2021
|
Looks good @gnespolino ! Thanks for the explanation/description. |
|
I think we could have added a few unit tests here, especially |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When we introduced possibility to include null values in filters, we needed to threat "IN" operator in a different way. Infact the way the code is structured didn't allow to render SQL condition properly when IN included also a null value.
In cases such ";IN;Female:NV", SQL condition needs to be "field IN (female) or field is null".
In this case a proper InQueryFilter class was written to be able to render condition properly. However all the work to encode and quote parameters properly, which came out of the box for other operators, need to be handled inside InQueryFilter to be effective. This PR addresses a missing quoting and escaping which was introduced when InQueryFilter was written.
This should go into 2.37.
Adding @Philip-Larsen-Donnelly for approval.