commondb.BackfillLowercaseEmails opens with SELECT id, email FROM users carrying no predicate,
filters the rows in Go, and runs at every authserver start through runStartupDataTasks. Once the
pass has converged it still reads every row of users on every boot to discover there is nothing
to do, and that cost grows with the table and never falls.
The doc comment on the function claims parity with the precedent it names, being "idempotent and
resumable, on BackfillEncryptedOTPSecrets' terms (#82)". That is true of the outcome and not of
the cost. commondb/otp_backfill.go filters with sb.Where(sb.NotEqual("otp_secret", "")), so
once #82's pass has run its predicate matches nothing and the query is free. This one has no
predicate to become free.
The Go-side filter is correct and is not what should change. The reason it exists is SQLite:
LOWER() maps ASCII only through modernc.org/sqlite, so a stored Ädmin@x.com is not selected
by any LOWER()-based predicate there and would survive the pass untouched. That is the exact
defect #283 decision 13 closed by deciding the whole rule in Go.
What makes this a real question rather than a one-line fix: on MySQL, PostgreSQL and SQL Server the
collation is now case-sensitive and LOWER() is Unicode-aware, so WHERE email <> LOWER(email)
there is a correct superset of the rows the Go filter keeps, and it is empty once the pass has
converged. So the cheap remedy is a predicate on three engines and no predicate on the fourth,
which is a per-engine branch inside a pass whose entire design is that it does not have one. #283
decision 13 was answered A, "the data layer decides identity uniformly", by the repository
owner. Whether "uniformly" governs which rows are selected as well as how they are decided is
the question this issue is really asking, and it is the owner's.
Options, if it is worth doing at all:
- A. Per-engine predicate.
WHERE email <> LOWER(email) on MySQL, PostgreSQL and SQL Server;
the unfiltered scan on SQLite. Cheapest, and it introduces the engine branch decision 13 A reads
against.
- B. A converged marker. A settings column recording that the pass has run to completion, so it
is skipped entirely on later boots, following the shape of the one-shot data-key move. No engine
branch, at the price of a migration and a column.
- C. Leave it. The scan is two columns and no join, so on any users table this software is
plausibly deployed against it is milliseconds at boot and nothing else.
Not measured. The reviewer that raised it said so in its own verdict: the cost was reasoned
from the source rather than executed against a large table, and it graded the finding minor on
that basis. Anyone picking this up should measure before choosing, because C is a real answer.
Where: src/core/data/commondb/email_backfill.go, BackfillLowercaseEmails, the opening
sqlbuilder.NewSelectBuilder() block. The precedent to compare against is
src/core/data/commondb/otp_backfill.go, BackfillEncryptedOTPSecrets.
Provenance. Raised at the final review gate of #283 (PR #287), round 3, as finding 3, and
verified as accurate there. Taken as a follow-up rather than fixed because the remedy is a choice
rather than forced: see the options above. The full account is in
docs/issue-283-collation-parity/closing.md, section 10.
Related. #82 is the precedent whose filtered predicate this one lacks. #262 tracks removing the
two one-shot startup conversions in NewDatabase; BackfillLowercaseEmails is a third startup
pass that #262 does not cover, and if it is ever given a converged marker (option B) it becomes a
candidate for the same treatment.
commondb.BackfillLowercaseEmailsopens withSELECT id, email FROM userscarrying no predicate,filters the rows in Go, and runs at every authserver start through
runStartupDataTasks. Once thepass has converged it still reads every row of
userson every boot to discover there is nothingto do, and that cost grows with the table and never falls.
The doc comment on the function claims parity with the precedent it names, being "idempotent and
resumable, on
BackfillEncryptedOTPSecrets' terms (#82)". That is true of the outcome and not ofthe cost.
commondb/otp_backfill.gofilters withsb.Where(sb.NotEqual("otp_secret", "")), soonce #82's pass has run its predicate matches nothing and the query is free. This one has no
predicate to become free.
The Go-side filter is correct and is not what should change. The reason it exists is SQLite:
LOWER()maps ASCII only throughmodernc.org/sqlite, so a storedÄdmin@x.comis not selectedby any
LOWER()-based predicate there and would survive the pass untouched. That is the exactdefect #283 decision 13 closed by deciding the whole rule in Go.
What makes this a real question rather than a one-line fix: on MySQL, PostgreSQL and SQL Server the
collation is now case-sensitive and
LOWER()is Unicode-aware, soWHERE email <> LOWER(email)there is a correct superset of the rows the Go filter keeps, and it is empty once the pass has
converged. So the cheap remedy is a predicate on three engines and no predicate on the fourth,
which is a per-engine branch inside a pass whose entire design is that it does not have one. #283
decision 13 was answered A, "the data layer decides identity uniformly", by the repository
owner. Whether "uniformly" governs which rows are selected as well as how they are decided is
the question this issue is really asking, and it is the owner's.
Options, if it is worth doing at all:
WHERE email <> LOWER(email)on MySQL, PostgreSQL and SQL Server;the unfiltered scan on SQLite. Cheapest, and it introduces the engine branch decision 13 A reads
against.
is skipped entirely on later boots, following the shape of the one-shot data-key move. No engine
branch, at the price of a migration and a column.
plausibly deployed against it is milliseconds at boot and nothing else.
Not measured. The reviewer that raised it said so in its own verdict: the cost was reasoned
from the source rather than executed against a large table, and it graded the finding
minoronthat basis. Anyone picking this up should measure before choosing, because C is a real answer.
Where:
src/core/data/commondb/email_backfill.go,BackfillLowercaseEmails, the openingsqlbuilder.NewSelectBuilder()block. The precedent to compare against issrc/core/data/commondb/otp_backfill.go,BackfillEncryptedOTPSecrets.Provenance. Raised at the final review gate of #283 (PR #287), round 3, as finding 3, and
verified as accurate there. Taken as a follow-up rather than fixed because the remedy is a choice
rather than forced: see the options above. The full account is in
docs/issue-283-collation-parity/closing.md, section 10.Related. #82 is the precedent whose filtered predicate this one lacks. #262 tracks removing the
two one-shot startup conversions in
NewDatabase;BackfillLowercaseEmailsis a third startuppass that #262 does not cover, and if it is ever given a converged marker (option B) it becomes a
candidate for the same treatment.