SMF
SMF copied to clipboard
SimpleMachines
Allow to override the default behavior of db case sensitive
With pg 12 release you can use collation which are case insensitive also in mysql you can use collation which are case sensitive, to give the admin a chance to change the default behavior i change some stuff around this.
pg12 release note: https://www.postgresql.org/docs/12/collation.html#COLLATION-NONDETERMINISTIC
I have some questions here....
First, why is it an admin setting? Shouldn't the collation be queried? As soon as you make it a setting, someone could set it wrong... I'd rather query it and avoid mistakes.
Another note - the upgrader also has a copy of settings.php in it that must be updated any time we change settings.php.
collation can be set per column, to check all everytime would be not worth by looking of price of performance lost.
why we got multiple version of the settings.php file?
If it can be set once per column, then a single setting in settings.php won't work, will it?
What does the setting mean - which column does it refer to?
I'm trying to understand what this is going to do for us...
it's the way how smf work, you can set on every column if this is a utf8 column or not, in smf you set it once for all in the same spirit this config paramter is created/define.
SMF (mysql, anyway) has always been case-insensitive. Is this intended to allow an admin to turn that off? Is that really needed?
Has SMF (pg) been hard-coded to be case sensitive to-date? If so, wouldn't it be preferable to simply make pg case-insensitive & leave everything alone?
I don't understand why we need a configuration setting. I would think we'd want to keep SMF entirely case-insensitive, as it always has been.
in mysql the admin can choose to use a cs collation, nothing new here, only smf would react in this case wrong, since smf believe mysql is always ci.
would be nice if you look into the change before you ask this kind of question, yes it's hardcoded, untile pg 12 (which got released today) only cs exists.
Then look where the cs check is place to understand the thing.
Just trying to understand here.
I really don't think we want an unnecessary setting.
If pg is messing up some comparisons, e.g., names, due to capitalization, I'd rather fix that bug by declaring the column with a citext than adding an unnecessary setting.
I think we should focus on the actual bug, not adding settings that should never be used.
If we wanted to provide the capability to enable or disable case-sensitivity, that would be a valid reason to add a setting. I am not sure there is a need to force case-sensitivity throughout. If I hear one, I am game for a setting.
Other than that, focus on the bug... (Like you did here: https://www.simplemachines.org/community/index.php?topic=559116.0)
Or think thru and share what the implementation might be like (if pg12 is detected, we do xxxx, otherwise yyyyy, and for mysql we xxxxxx).
As is, this PR implements an unused setting, which has no supporting use case.
The var is used and allow mysql how maybe set the db wrong to allow to keep what they got and only to change smf in a cs mode and on the other hand allowed pg12+ people to change the mode to ci für smf without changing the code.
@sbulen when i look into the rc3 issues, ther are no bugs, all entries are features request.
I agree with this not making 2.1. While the bug exists, this has very significant recursions of how we interact with the database. It is way to late in 2.1 to accept this. With the future SMF versions, this should be designed this way OOB and that way we can factor in any database abstraction layer changes to handling this properly.
My recommendation is we close this as won't fix and look for it for future SMF versions.
Note: This PR targets release-2.1, but it has been assigned to the milestones for SMF 3.0. The PR will need to be updated and resubmitted to target release-3.0. I am leaving it open for now so that it doesn't get forgotten.
