Skip to content

Add GIN expression index on boxel_index.types so type-anchored search filters are index-served #5533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
FadhlanR wants to merge 2 commits into
base: main
Choose a base branch
from
+75 −235
Open

Add GIN expression index on boxel_index.types so type-anchored search filters are index-served #5533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3001bf
Add GIN expression index on boxel_index.types for containment filters
FadhlanR Jul 17, 2026
127e57b
Make concurrent index builds self-healing on retry
FadhlanR Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
235 changes: 0 additions & 235 deletions packages/host/config/schema/1783724082538_schema.sql
View file
Open in desktop

This file was deleted.

0 ...st/config/schema/1783724976754_schema.sql → ...st/config/schema/1784272066344_schema.sql
View file
Open in desktop
File renamed without changes.
70 changes: 70 additions & 0 deletions packages/postgres/migrations/1784272066344_types-containment-gin-index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// GIN expression index for type-anchored search filters. The types-contains
// predicate in packages/runtime-common/expression.ts compiles to
// `COALESCE(types, '[]'::jsonb) @> '["<type>"]'::jsonb`, and the planner
// only uses an index whose expression matches the query expression — the
// index must cover COALESCE(types, '[]'::jsonb), not the bare column.
// jsonb_path_ops is smaller and faster than the default jsonb_ops and
// supports @>, the only operator this predicate uses.
Comment on lines +6 to +7

@habdelra habdelra Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] jsonb_path_ops is the right call for this predicate, and it's worth spelling out the two capabilities it trades away so a future change on this column knows when the choice stops fitting:

  1. It serves only @>. The key-exists operators (?, ?|, ?&) and everything else in the default jsonb_ops repertoire are unsupported. The types-contains predicate is pure containment, so nothing is lost — but a future filter wanting e.g. "does any type key start with…" or key-existence semantics would need a jsonb_ops index (or a different structure entirely).
  2. It produces no index entries for empty arrays/objects. Rows whose types is NULL index as '[]' under the COALESCE and therefore have no entries — which is exactly right: they can never satisfy a containment test against a non-empty right-hand side, and the engine always binds a one-element array ('["<key>"]'), never an empty one. The degenerate @> '[]' query (true for every row, answerable only by a full index scan) is not a shape the engine can emit.

Nothing to change — this is confirmation that both trade-offs are safe for the emitted predicate, plus the conditions under which they'd stop being safe.

//
// The bare-column GIN indexes on types (from 1735668047598 and
// 1735832183444) can never serve this predicate, and no other query
// filters on types, so they are dropped here.
//
// `boxel_index_working` mirrors `boxel_index` because the search path
// can target either table via the useWorkInProgressIndex query option.
//
// CONCURRENTLY avoids locking writes during long builds in production.
// node-pg-migrate's outer singleTransaction wrapper is opted out of via
// pgm.noTransaction() — CREATE/DROP INDEX CONCURRENTLY cannot run inside
// a transaction at all. node-pg-migrate logs
// `#> WARNING: Need to break single transaction! <` when applying this
// migration; that is expected, not a failure.
//
// An interrupted CONCURRENTLY build (e.g. a deploy restart mid-build)
// leaves an INVALID index under the target name, which the planner
// ignores. The migration only re-runs when a prior attempt failed to
// record, so each CREATE is preceded by an unconditional DROP to clear
// any such leftover and make retries self-healing.

exports.up = (pgm) => {
pgm.noTransaction();
pgm.sql(
`DROP INDEX CONCURRENTLY IF EXISTS boxel_index_types_containment_idx;`,
);
pgm.sql(`
CREATE INDEX CONCURRENTLY boxel_index_types_containment_idx
ON boxel_index
USING GIN ((COALESCE(types, '[]'::jsonb)) jsonb_path_ops);
`);
Comment on lines +35 to +38

@habdelra habdelra Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] The open question for an expression index like this is whether the planner still reaches it inside the full query the engine emits — an isolated EXPLAIN on the bare predicate doesn't prove that. Some context on why that's a real concern here, and the verification result:

What the engine actually emits. A type filter never appears as a single containment predicate. typeCondition in packages/runtime-common/index-query-engine.ts expands one card ref into an OR of containment arms — one per equivalent spelling of the type key (internalKeysFor: RRI form, real-URL form, virtual-alias form) — and _search ANDs that with i.realm_url = $n, the is_deleted check, i.type = $n, and a not-errored predicate that references the LEFT-JOINed prerendered_html row. Plural-path field filters additionally splice CROSS JOIN LATERAL jsonb_tree(...) into the FROM clause, and everything runs under GROUP BY i.url, i.type + ORDER BY ... LIMIT. Any of those wrappers could in principle keep the qual from being pushed into the scan of i.

Verified against a real indexed dataset (13.5k rows / 23 realms, Postgres 16.3), running the full emitted _search shape rather than the bare predicate. The planner pushes the whole OR into the scan as a BitmapOr over this index, and BitmapAnds it with the realm/type btree:

Bitmap Heap Scan on boxel_index i  (actual rows=659)
  Recheck Cond: ((COALESCE(types,'[]'::jsonb) @> '["<url form>"]' OR
                  COALESCE(types,'[]'::jsonb) @> '["<rri form>"]')
                 AND realm_url = $realm AND type = 'instance')
  ->  BitmapAnd
        ->  BitmapOr
              ->  Bitmap Index Scan on boxel_index_types_containment_idx (rows=659)
              ->  Bitmap Index Scan on boxel_index_types_containment_idx (rows=0)
        ->  Bitmap Index Scan on boxel_index_realm_url_type_index (rows=1186)

The same holds in the three variations that could have defeated it:

  • Generic plan with bind parameters (EXPLAIN (GENERIC_PLAN) with $n placeholders — the extended-protocol worst case, relevant because node-postgres sends parameterized statements): index cond binds COALESCE(types, '[]'::jsonb) @> $3 directly.
  • With a jsonb_tree lateral join present (a plural-path eq alongside the type filter): the containment qual still lands in the scan of i, before the lateral fan-out.
  • The hasFileType / hasInstanceType SELECT 1 ... LIMIT 1 shape: index-served, sub-ms.

And the before-state confirms the premise for dropping the bare-column indexes: with only boxel_index_types_index present, the planner scans via the realm/type btree and evaluates the containment row-by-row as a post-scan filter (Rows Removed by Filter: 527) — the bare GIN never appears in any plan.

One boundary worth knowing: the index only helps where the containment sits in an AND chain over i columns. A type condition nested under a filter-level any: whose sibling branch references a json_tree alias becomes an OR spanning the lateral join, which no index can serve — that's inherent to OR-across-a-join, and the dominant shapes (the on type anchor and card-type filters, which are always AND-ed) are the ones served.

For background on why no query-side change is needed here (unlike the singular string eq filter, which had to be rewritten from search_doc -> 'a' ->> 'b' = $n extraction into the JsonContains search_doc @> '{...}'::jsonb containment node to become GIN-servable): GIN jsonb opclasses serve @>, and field-extraction predicates are not GIN-servable at all. The types predicate is already containment-shaped — the only mismatch was on the index side, which this migration fixes.

pgm.sql(
`DROP INDEX CONCURRENTLY IF EXISTS boxel_index_working_types_containment_idx;`,
);
pgm.sql(`
CREATE INDEX CONCURRENTLY boxel_index_working_types_containment_idx
ON boxel_index_working
USING GIN ((COALESCE(types, '[]'::jsonb)) jsonb_path_ops);
`);
pgm.sql(`DROP INDEX CONCURRENTLY IF EXISTS boxel_index_types_index;`);
pgm.sql(`DROP INDEX CONCURRENTLY IF EXISTS boxel_index_working_types_index;`);
};

exports.down = (pgm) => {
pgm.noTransaction();
pgm.sql(
`DROP INDEX CONCURRENTLY IF EXISTS boxel_index_types_containment_idx;`,
);
pgm.sql(
`DROP INDEX CONCURRENTLY IF EXISTS boxel_index_working_types_containment_idx;`,
);
// Restore under the original auto-generated names so the down migrations
// of 1735668047598 and 1735832183444 still find them. Same
// drop-then-create pattern as up() to clear INVALID leftovers on retry.
Comment on lines +59 to +61

@habdelra habdelra Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Verified the restore fidelity: the referenced migrations create these via pgm.createIndex(table, 'types', { method: 'gin' }), whose auto-generated names are exactly boxel_index_types_index / boxel_index_working_types_index with the default jsonb_ops opclass on the bare column — which is what's recreated here, so their down()s (pgm.dropIndex(table, 'types')) resolve by name. 👍

One wording nit while you're in here: "still find them" is relative-time phrasing — the comment reads cleaner as a timeless contract, e.g. "…so the down migrations of 1735668047598 and 1735832183444 find them by name." (Referencing migrations by timestamp is fine — they're immutable filenames in this directory, unlike tracker or PR numbers, which don't belong in comments.)

pgm.sql(`DROP INDEX CONCURRENTLY IF EXISTS boxel_index_types_index;`);
pgm.sql(
`CREATE INDEX CONCURRENTLY boxel_index_types_index ON boxel_index USING GIN (types);`,
);
pgm.sql(`DROP INDEX CONCURRENTLY IF EXISTS boxel_index_working_types_index;`);
pgm.sql(
`CREATE INDEX CONCURRENTLY boxel_index_working_types_index ON boxel_index_working USING GIN (types);`,
);
};
5 changes: 5 additions & 0 deletions packages/runtime-common/expression.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,11 @@ export function expressionToSql(
.map(renderElement)
.join(' ');
}
// The boxel_index_types_containment_idx GIN indexes (migration
// 1784272066344) cover this exact expression — Postgres only uses an
// expression index when the query expression matches it verbatim, so
// changing the SQL here (e.g. the COALESCE wrapper) un-indexes type
// filters unless the index expression moves with it.
Comment on lines +557 to +561

@habdelra habdelra Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Good — this comment is the guard that keeps the two sides of the contract from drifting, since nothing executable enforces the coupling. One clarification worth folding in: "matches it verbatim" slightly overstates the strictness, and knowing where the real line sits helps a future editor judge what's safe.

The planner matches on the parsed expression tree, not the SQL text. So these are all fine and don't break index use:

  • the alias qualification (i.types here vs. bare types in the index DDL),
  • whitespace, keyword case, COALESCE vs coalesce,
  • the right-hand side of @> (it's the query value — only the left operand must match the indexed expression).

What does break it is any semantic change to the left operand's tree: a different empty-array placeholder ('{}'::jsonb), an added cast, or restructuring into types IS NOT NULL AND types @> …. The failure mode is silent — queries stay correct, they just fall back to evaluating containment row-by-row behind the realm/type btree.

Suggested tweak: "…Postgres only uses an expression index when the query's left operand structurally matches the indexed expression (aliasing and whitespace don't matter; any semantic change to the COALESCE wrapper does), so changing the SQL here un-indexes type filters unless the index expression moves with it."

return ['COALESCE(', column, `, '[]'::jsonb) @>`, param([key]), '::jsonb']
.map(renderElement)
.join(' ');
Expand Down
Loading