From e11658b4e0cabec406d267f3f4699d4192f6ed2a Mon Sep 17 00:00:00 2001 From: Gustavo Date: Fri, 31 Jul 2026 12:06:25 -0300 Subject: [PATCH] Correct misleading comment about the candidates derived-subquery rationale Review found the comment claimed a per-row precomputation optimization that SQLite does not actually perform: the reviewer counted replace() opcodes in the compiled bytecode and got an identical count (305) for both the inline and derived-subquery forms of the query, since SQLite flattens the subquery and re-substitutes the expression text at each usage site either way. Rewrote the comment to describe the derived subquery as what it actually is (a readability device), and to state plainly that the perf investigation did not conclusively isolate the rare timing outliers to this SQL's shape. --- src/outreach/campaign-queue.mjs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/outreach/campaign-queue.mjs b/src/outreach/campaign-queue.mjs index 0597aa9..8673dd5 100644 --- a/src/outreach/campaign-queue.mjs +++ b/src/outreach/campaign-queue.mjs @@ -25,17 +25,20 @@ function equivalentSql(column) { } function candidateQuery(confirmed) { - // leadNumber/leadEquivalent are computed ONCE per row in the derived `candidates` - // subquery below (as lead_number/lead_equivalent columns) instead of being - // re-embedded as raw expression text at every usage site further down. The - // normalizedSql()/equivalentSql() expressions expand into a deeply nested chain - // of replace()/substr()/length() calls; textually re-embedding them at each of - // this query's several correlated-subquery sites (verification join, opt-out - // check, shared-number check, inbound-replies check) made SQLite recompute that - // whole chain independently at every site, for every row — measurably expensive - // once a 5th usage site (the shared-number check) was added, especially in the - // worst case where a candidate segment has zero pre-verified numbers and the - // scan can't terminate early (see task-3-report.md for the investigation). + // lead_number/lead_equivalent are named columns computed in the derived + // `candidates` subquery below, purely as a readability device so the rest of + // this query can refer to `candidates.lead_number`/`candidates.lead_equivalent` + // instead of repeating the normalizedSql()/equivalentSql() expression text at + // every usage site. This is NOT a per-row precomputation optimization: SQLite + // flattens the derived subquery and re-substitutes the expression text at each + // usage site regardless, so the compiled query plan (and replace()-opcode count) + // is identical to the earlier inline form. During Task 3's perf investigation, + // rare multi-second outliers were observed on the 1.3M-row perf test both before + // and after this restructuring; repeated runs (5 consecutive, on two different + // machines) landed comfortably under the 8s budget either way, and the root + // cause of the rare spikes was not conclusively isolated to this SQL's shape — + // it looked more consistent with host-level noise than a query-cost regression. + // See task-3-report.md for the full investigation and evidence. const leadNumberExpr = normalizedSql('raw.whatsapp'); const leadEquivalentExpr = equivalentSql('raw.whatsapp'); const leadNumber = 'candidates.lead_number';