Skip to content

[3.0] Case insensitive comparisons (part 1 of 2) — a {ci:} type for the query language - #9596

Open
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/db-ci-token
Open

[3.0] Case insensitive comparisons (part 1 of 2) — a {ci:} type for the query language#9596
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/db-ci-token

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

Description

Whether a string comparison folds case is decided by the database engine. MySQL folds
it in the column's collation; PostgreSQL compares exactly. Today every caller that cares
has to know this, read Db::$db->case_sensitive, and wrap the column in LOWER() itself:

'real_name' => Db::$db->case_sensitive ? 'LOWER(real_name)' : 'real_name',

That puts the decision at each call site and defaults to the wrong answer on PostgreSQL
when somebody does not think to add it. It also fails quietly — the query returns fewer
rows rather than erroring, so nothing reaches smf_log_errors. #9592, #9593 and #9594 are
all that shape.

This moves the decision into the query string, where the substitution layer already lives.

  • {ci:column} expands to column on MySQL and LOWER(column) on PostgreSQL.
    The column is named inline rather than through $db_values, so a comparison shows in
    the query text which column it folds. Only a column name, optionally qualified by a
    table alias, is accepted.
  • {ci_string:key} is the matching value type, for the two places that were folding
    the value in SQL rather than in PHP.

Then converts the call sites that were already branching on Db::$db->case_sensitive.

No behaviour change. Each conversion expands to exactly what the ternary it replaces
produced, on both engines. Two things worth calling out:

  • User::addQueryCustomizationsForLoadType() still reads case_sensitive for its
    parameters, because an {array_string:} list has no {ci:} of its own. Only the
    duplicated WHERE clause goes away. An {array_ci_string:} could follow if it turns
    out to be wanted anywhere else.
  • Memberlist keeps its own LOWER() loop. It folds expressions such as
    COALESCE(group_name, {string:blank_string}) as well as plain columns, and an
    expression is not what {ci:} accepts. Only its value side is converted.

Not reachable from the unit suite: replacement__callback() is protected and reached
through quote(), which needs a live connection to escape with. Part 2 adds the test
that is possible without one — a guard over the source files, so that a new comparison
written the old way fails CI instead of being found years later.

Verified by hand on both engines from the Docker environment, and composer lint and
vendor/bin/phpunit are clean.

Issues References (Fixes|Related|Closes)

  1. Related: [3.0]: Email bans do not match members with uppercase in their address on PostgreSQL #9592
  2. Related: [3.0]: Searching by user name is case-sensitive on PostgreSQL #9593
  3. Related: [3.0]: Two accounts can register on the same email address on PostgreSQL #9594
  4. Related: Hardcoded case insensitive MySQL #3214case_sensitive being a per-engine constant rather than a property of the column

Whether a string comparison folds case is decided by the database engine:
MySQL folds it in the column's collation, PostgreSQL compares exactly. Callers
handled that themselves by reading Db::$db->case_sensitive and wrapping the
column in LOWER(), which put the decision at every call site and left it out
wherever somebody did not think to add it.

{ci:column} moves it into the query string, where the substitution layer
already lives. It expands to the bare column on MySQL and to LOWER(column) on
PostgreSQL. {ci_string:key} is the matching value type, for the places that
were folding the value in SQL rather than in PHP.

The column is named inline rather than through $db_values, so that a
comparison shows in the query text which column it folds. Only a column name,
optionally qualified by a table alias, is accepted.

Memberlist keeps its own LOWER() loop, because it folds expressions such as
COALESCE(group_name, '') as well as plain columns, and those are not what
{ci:} accepts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant