Remove NOT IN unstitched guard from facets no-filter path
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 <freben@spotify.com>
Made-with: Cursor
This commit is contained in:
@@ -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'],
|
||||
|
||||
@@ -676,20 +676,12 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog {
|
||||
}
|
||||
|
||||
async facets(request: EntityFacetsRequest): Promise<EntityFacetsResponse> {
|
||||
// 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<DbSearchRow>('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');
|
||||
|
||||
Reference in New Issue
Block a user