-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
FEATURE: Case-insensitive property value criteria #4725
FEATURE: Case-insensitive property value criteria #4725
Conversation
Streamlines the behavior of the `ContentSubgraph` for string based PropertyValueCriteria to be case sensitive (to be in sync with Neos < 8) Related: #4721
Note this is a bit complex topic, I know eel / fizzle separates between case(in)sensitive but if the implementation is correct in all cases is another question, also |
Adds support for the special case _insensitive_ operators: * `=~` equals * `$=~` ends with * `^=~` starts with * `*=~` contains Related: #2600
Got to block this for a second here, I think this uncovers a different problem we should fix first and apply the solution to this as well. I started to investigate a bit and the events table does use "proper" which seems missing from the schema construction we do for the contentrepository projections and I think we should definitely add it otherwise the charset and collaction are probably left to some system default which would be a bad idea. See eg. here: Flow does set:
So that seems to fit. IMHO we should first change all projections to actually adhere to this and then also use this to decide which collation to use in queries like these here. Edit to make this clearer, my local system also has the projection tables on |
How can we continue here? The above has been fixed. |
Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentSubgraph.php
Outdated
Show resolved
Hide resolved
With db64f78 I hard-coded the case-sensitive search to collate properties and search term to And since it is all covered with tests, this can me merged IMO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks looks good by reading ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for "bear"ing with me (SCNR), this looks great.
Adds support for the special case insensitive operators:
=~
equals$=~
ends with^=~
starts with*=~
containsAnd makes sure, that the other string based operators (
=
,$=
,^=
and*=
) are case sensitive on all supported platforms.Resolves: #4721
Related: #2600