perf(core): drive folded term hydration from the pivot on SQLite (#1722)#1726
Conversation
…sh-cms#1722) `foldedHydrationSelects` folded the per-entry taxonomy-term hydration as a plain `JOIN`. On stats-blind SQLite/D1 (D1 never maintains `sqlite_stat1`) the planner drove the correlated subquery from `taxonomies` by locale, enumerating every term in the locale and probing the pivot once per emitted row — tens of thousands of rows per list page on large-taxonomy sites. Pin the join order with `CROSS JOIN` on the SQLite path so the subquery drives from the `content_taxonomies` pivot by `(collection, entry_id)` and probes `taxonomies` by `translation_group`. Postgres keeps statistics and rejects `CROSS JOIN … ON`, so it stays a plain `JOIN` there. The byline subquery gets the same hardening for consistency (its composite `(translation_group, locale)` unique index already yields the good plan). Adds an EXPLAIN QUERY PLAN regression test asserting the real loader query drives from the pivot, not `idx_taxonomies_locale`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 9e14c66 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
area/core
size/M
review/needs-review
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approach judgment: this is a focused, well-motivated fix for a real D1/SQLite query-plan regression. Pinning the join order with CROSS JOIN on SQLite is the idiomatic way to stop a stats-blind planner from driving a correlated hydration subquery from the wrong table, and the change is byte-identical on Postgres. Adding a regression test that asserts the plan shape is exactly the right shape of test for this kind of issue.
What I checked:
- The diff: only
loader.ts, the newloader-fold-plan.test.ts, and a changeset. - SQL safety: identifiers use
sql.ref(), values are parameterized; no injection risk. - Locale filtering: the subqueries still correlate on
t.locale = ${o}.localeandb.locale = ${o}.locale, so per-entry locale semantics are preserved. - Index/schema context: the
idx_taxonomies_translation_groupandidx__emdash_bylines_translation_group(and the partialidx_bylines_group_locale_unique) indexes exist, so the pivot-driven plan the CROSS JOIN enforces is realizable. - Existing test coverage:
loader-fold.test.tsalready exercises folded output on both dialects; the new test adds the plan-shape guard without duplicating output assertions. - Changeset: present and user-facing.
Headline conclusion: the code change is correct and the test effectively guards the regression. I have only minor feedback: a misleading comment about Postgres and a chance to broaden the plan test slightly.
MA2153
changed the title
Extend loader-fold-plan to assert the pivot-driven plan for the byline subquery and the loadEntry path, not just taxonomy-on-loadCollection — both share the CROSS JOIN join-order hardening from emdash-cms#1722. The byline composite (translation_group, locale) index already yields the good plan, so this guards against a future index change regressing it. Confirmed non-vacuous: forcing a plain JOIN on the SQLite path fails both paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
size/L
and removed
review/needs-review
github-actions
Bot
added
the
review/needs-rereview
There was a problem hiding this comment.
This PR is a focused, well-motivated performance fix for a real D1/SQLite query-plan regression. Pinning the join order with CROSS JOIN on SQLite is the idiomatic way to stop a stats-blind planner from driving a correlated hydration subquery from the wrong table; the Postgres path remains byte-identical. The regression test asserts the plan shape, which is the right kind of test for this category of bug.
I re-read packages/core/src/loader.ts, the new packages/core/tests/integration/loader-fold-plan.test.ts, the changeset, and the relevant migrations/indexes. The SQL is safe (sql.ref for identifiers, parameterized values), locale filtering is preserved in both subqueries (t.locale = ${o}.locale / b.locale = ${o}.locale), and the asserted indexes (idx_taxonomies_translation_group, idx_bylines_group_locale_unique) exist. The test runs the real loader query on a stats-blind in-memory SQLite DB and guards both loadCollection and loadEntry for both folded subqueries.
From the prior review: the Postgres comment is accurate (Postgres grammar does not allow CROSS JOIN ... ON), and the plan test now covers both call sites and both subqueries, so the earlier minor feedback is satisfactorily addressed. No blocking issues remain.
github-actions
Bot
added
review/approved
2 participants
What does this PR do?
Fixes a query-plan regression in folded taxonomy-term hydration.
foldedHydrationSelects(packages/core/src/loader.ts) folds per-entry term hydration into the content query as a correlated JSON-array subquery using a plainJOIN. On stats-blind SQLite/D1 (D1 never runsANALYZE/ maintainssqlite_stat1), the planner is free to drive that subquery fromtaxonomiesby locale — enumerating every term in the locale and probing the pivot once per emitted row:On a site with ~1,540 terms in the active locale, a single unfiltered 25-row list page reads ≈ 25 × 1,540 ≈ 38.5K rows — the floor for every list page, paid on every cache miss.
The fix pins the join order with
CROSS JOINon the SQLite path so the subquery always drives from thecontent_taxonomiespivot by(collection, entry_id)and probestaxonomiesbytranslation_group— a handful of reads per entry, independent of taxonomy size and of statistics:Postgres keeps statistics and rejects
CROSS JOIN … ON, so it remains a plainJOINthere (byte-identical SQL to before). The byline subquery in the same function gets the sameCROSS JOINhardening for consistency and future-proofing — its composite(translation_group, locale)unique index already yields the pivot-driven plan today, so this is a no-op there but guards against future index changes.Closes #1722
Type of change
Checklist
pnpm typecheckpassespnpm lintpasses (0 diagnostics)pnpm testpasses (targeted: 548 tests across loader/query/fold/taxonomy/byline suites)pnpm formathas been runmessages.pochanges includedemdashpatch)AI-generated code disclosure
Screenshots / test output
New regression test (
packages/core/tests/integration/loader-fold-plan.test.ts) captures the real loader query, runsEXPLAIN QUERY PLANon a stats-blind (D1-shaped) SQLite DB, and asserts the taxonomy subquery drives from the pivot (idx_taxonomies_translation_group) rather than scanningidx_taxonomies_locale. It was written test-first: it failed against the old plain-JOINcode and passes after the fix.🤖 Generated with Claude Code