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

[albertlast]

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.
• {string_ci:key} and {array_string_ci:key} are the matching value types.
  The suffix rather than a prefix, because array_ already occupies the prefix position
  on the existing types, so {array_string_ci} composes where {array_ci_string} would
  not.

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

Every conversion expands to exactly what the ternary it replaces produced, on both
engines, with one deliberate exception:

• User::addQueryCustomizationsForLoadType() changes behaviour. It used to fold the
  names with strtolower() in PHP and compare them against a column folded by SQL
  LOWER(). Those disagree outside ASCII — ÄNNA folds to änna on the column and to
  Änna in PHP — so loading that member by name never matched on PostgreSQL. It now
  hands the list to {array_string_ci:} and both sides fold the same way. This is the
  only behaviour change in the branch, and it is the last caller of
  Db::$db->case_sensitive outside Memberlist.
• 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 #3214 — case_sensitive being a per-engine constant rather than a property of the column

[jdarwood007]

I'm not certain if it should be {ci_string} or {string_ci}. The later seems like it becomes a suffix of it and would easily add to additional options in the future.

We should support as well {array_string_ci}

[jdarwood007]

Shouldn't this use {ci_string}?

[albertlast]

Both done, and you are right about the suffix for a reason I had not spotted: array_ is already a prefix modifier on the existing types (array_int, array_string, array_uuid, array_inet). So a ci prefix would have forced the list form to be {array_ci_string} or {ci_array_string}, both of which read badly. As a suffix it slots in beside what is there:

{string}        {string_ci}
{array_string}  {array_string_ci}

Pushed as 6fb2bde. {ci_string} is now {string_ci}, and {array_string_ci} folds each value in the list the way {string_ci} folds one.

Adding the array type paid for itself immediately: User::addQueryCustomizationsForLoadType() was the one call site still reading Db::$db->case_sensitive, because an IN list had no folded form to hand its values to. It now reads:

$query_customizations['where'][] = '{ci:mem.member_name} IN ({array_string_ci:users})';
$query_customizations['params']['users'] = $users;

That is a behaviour change, and the only one in the branch, so flagging it rather than burying it. The old code folded the names with strtolower() in PHP and compared them against a column folded by SQL LOWER(). Those disagree outside ASCII — a member named ÄNNA gives änna on the column and Änna from strtolower(), so loading that member by name never matched on PostgreSQL. Both sides now fold the same way. Happy to split it into its own commit on top if you would rather the branch stayed strictly behaviour-preserving.

One naming question I did not want to decide for you: I left the column type as {ci:column} rather than renaming it to {column_ci:}. It is not a variant of an existing value type the way {string_ci} is — it names a column inline and takes no $db_values key — so the suffix convention does not obviously apply to it, and it is the type that appears most often. Say the word if you would prefer the symmetry.

#9597 follows the rename, and User.php drops out of the second list there as a result of the above.

[jdarwood007]

I would like to see some feedback from @Sesquipedalian on this. Its the right way to go codewise to ensure we handle this. But this PR does not address the database performance side of the equation. We still need to address that. Each one of these columns we are natively handling here and has a index in pg, needs that index to be ci transformed.

This does create a mismatch in the index creation between mysql and pg, but for performance reasons it should be accepted.

[jdarwood007]

And don't worry on the behaviour change. It sounds like that is a case where we didn't handle UTF-8 properly and now that we have the proper extensions, we are actually handling the query right.

[albertlast]

Agreed that this branch does not touch the index side. I measured it rather than guessing, on PostgreSQL 17.10 with 200,000 rows, and one of the three findings narrows the problem quite a lot.

1. member_name and real_name are already covered

The PostgreSQL-only indexes we create today (Sources/Db/Schema/v3_0/Members.php:517-531) are LOWER(member_name) and LOWER(real_name) with varchar_pattern_ops. That opclass serves every operator {ci:} produces, which I did not expect — it is usually described as the prefix-matching one:

query	plan	time
LOWER(name) = 'user12345'	Index Scan	0.17 ms
LOWER(name) IN ('user12345','user2')	Bitmap Heap Scan	0.21 ms
LOWER(name) LIKE 'user123%'	Bitmap Heap Scan	0.64 ms

So the two columns carrying the autosuggest, the member lookup and the PM recipient resolution need no index work at all.

2. email_address is the real gap, and it is a regression that predates this PR

idx_email_address is created for both engines, on the raw column (Members.php:381-389). A folded comparison cannot use it:

query	plan	time
email = 'user12345@example.com'	Index Scan using idx_email_address	0.101 ms
LOWER(email) = 'user12345@example.com'	parallel Seq Scan	43.4 ms

About 430x, on the registration duplicate-email check and on every profile email change.

To be fair about where it came from: it arrived in 959007f, which changed email_address = {string:email_address} to {raw:email_address_field} with the LOWER() ternary. This PR only carries it forward as {ci:email_address}, which expands to the same thing. MySQL is unaffected either way, since {ci:} leaves the column bare there.

3. Three ways to close it, two of them measured equivalent

• A functional index, LOWER(email_address) varchar_pattern_ops, in the PostgreSQL-only block beside the two that are already there → 0.119 ms. This is what you are proposing, and it works.
• A stored generated column with a plain index → 0.128 ms. Same result, but no index divergence between the engines, and we already have GeneratedColumn in the schema layer (Messages.php uses it). Worth weighing, because the functional index is the less portable of the two: MySQL needs 8.0.13 for functional indexes and MariaDB does not support them at all, whereas an indexed generated column works on all three.
• Normalising the address on write. Then the comparison goes back to a plain = on the index we already create — 0.101 ms, no new index, no divergence, and nothing to keep in sync.

The third one is worth reading next to @Sesquipedalian's comment on #9592, because it answers both questions with one change. He argued there that we should fold the domain part of an address and stop folding the local part, on RFC 5321 grounds. If addresses are normalised on write, the query stops folding anything at run time, which settles the correctness question he raised and the performance question you are raising at the same time.

The other columns

group_name (membergroups is tiny) and website_title / website_url (admin member search only) need nothing. pm.from_name has no index on it at all, before or after this branch, so searching personal messages by author was always a scan — worth knowing, but not a regression. poster_name will want the same look when #9593 is fixed.

One that no index solves: LIKE '%term%' with a leading wildcard cannot use a btree at all, 50 ms measured. That is the memberlist and the admin member search. It is independent of case folding and would need pg_trgm.

I am happy to add the index in whichever form you two prefer, either here or as a follow-up. Holding off until you and @Sesquipedalian have weighed in, since the third option would make the first two unnecessary.

[Sesquipedalian]

Regarding {ci:...} vs. {column_ci:...}, although adding the column_ prefix to the token name isn't strictly necessary, it does make the meaning of the token clearer, so let's do that.

Regarding the email addresses, here's what I want to do:

• Store the correct form of the email address (i.e. with the local part allowing mixed case while the domain is case-folded) in the email_address column in the members table.
• Add a new email_address_ci column to the members table that stores a fully case-folded version of the email address.
• Change all existing database queries that search in email_address so that they search in email_address_ci.
• Continue to use the data stored in email_address when retrieving the email address for use.

Basically, these last two points amount to using email_address_ci in WHERE clauses but using email_address in SELECT clauses (e.g. SELECT email_address WHERE email_address_ci = {string:email}).

However, this plan goes beyond the scope of this pull request. So I think that this pull request can proceed with the WHERE {column_ci:email_address} = {string:email_address} pattern and just not worry about indexing that column for now. Then after this pull request has been merged, I will follow up with another pull request to implement my plan.

[Sesquipedalian]

In case it wasn't clear, @albertlast, once the change from {ci:...} to {column_ci:...} has been implemented, I will gladly merge this PR. Do you think you can make that change soon?

[albertlast]

In ~ 8h I could deliver the change.