From 0ecb700addca54aaff2765a73207aaff897db39b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 20 Apr 2026 17:31:00 +0200 Subject: [PATCH] Remove NOT IN unstitched guard from facets no-filter path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NOT IN guard against null final_entity rows was preventing the query planner from using parallel workers and (with a future covering index) index-only scans. Without the covering index it caused a 2.6x regression on the no-filter path (7s -> 18.4s). The filtered path already excludes unstitched entities via the whereNotNull('final_entities.final_entity') in the inner entityIdSubquery, so no guard is needed there. The no-filter path now matches 1.49.x behavior. A followup migration adding a covering index can re-introduce the guard efficiently. Signed-off-by: Fredrik Adelöw Made-with: Cursor --- .../src/service/DefaultEntitiesCatalog.test.ts | 17 +++-------------- .../src/service/DefaultEntitiesCatalog.ts | 12 ++---------- 2 files changed, 5 insertions(+), 24 deletions(-) diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts index 011edac291..e714d6e3a7 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts @@ -2452,7 +2452,7 @@ describe('DefaultEntitiesCatalog', () => { } it.each(databases.eachSupportedId())( - 'excludes not-yet-stitched entities, %p', + 'excludes not-yet-stitched entities from filtered facets, %p', async databaseId => { await createDatabase(databaseId); @@ -2500,19 +2500,8 @@ describe('DefaultEntitiesCatalog', () => { stitcher, }); - // Without filters: unstitched entity should be excluded - await expect( - catalog.facets({ - facets: ['metadata.name'], - credentials: mockCredentials.none(), - }), - ).resolves.toEqual({ - facets: { - 'metadata.name': [{ value: 'stitched', count: 1 }], - }, - }); - - // With filter: unstitched entity should also be excluded + // With filter: unstitched entity should be excluded because the + // inner entityIdSubquery requires final_entity IS NOT NULL await expect( catalog.facets({ facets: ['metadata.name'], diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index 71a69b0100..9a6fdd595b 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -676,20 +676,12 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { } async facets(request: EntityFacetsRequest): Promise { - // Exclude not-yet-stitched (or future tombstoned) entities by - // anti-joining the tiny set of null final_entity rows. This is - // nearly free (~3k rows) compared to confirming ~520k non-null rows. - const unstitchedSubquery = this.database('final_entities') - .select('final_entities.entity_id') - .whereNull('final_entities.final_entity'); - const query = this.database('search') .whereIn( 'search.key', request.facets.map(f => f.toLocaleLowerCase('en-US')), ) .whereNotNull('search.original_value') - .whereNotIn('search.entity_id', unstitchedSubquery) .select({ facet: 'search.key', value: 'search.original_value', @@ -701,8 +693,8 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { // Build a subquery that finds matching entity IDs via // final_entities, so that the EXISTS-based filters correlate // against one-row-per-entity rather than the much larger search - // table. This keeps the facets aggregation fast even with many - // filter clauses or permission conditions. + // table. The whereNotNull guard on final_entity excludes + // not-yet-stitched (or future tombstoned) entities. const entityIdSubquery = this.database('final_entities') .select('final_entities.entity_id') .whereNotNull('final_entities.final_entity');