From c2de113030623feab2ec2c786ae94141d39f6a72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sat, 9 May 2026 22:45:43 +0200 Subject: [PATCH] draft(catalog-backend): drive queryEntities ordering from search-by-key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the LEFT OUTER JOIN + DISTINCT in the queryEntities CTE with an INNER JOIN that drives from the search table for the sort field's key. Entities lacking the sort field are excluded from both the result and the count, aligning totalItems with navigable entities. Removes DISTINCT (prerequisite: search table dedup migration). Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Fredrik Adelöw --- .changeset/query-entities-inner-join.md | 7 ++++ .../service/DefaultEntitiesCatalog.test.ts | 41 ++++++++++--------- .../src/service/DefaultEntitiesCatalog.ts | 34 ++++++++------- 3 files changed, 48 insertions(+), 34 deletions(-) create mode 100644 .changeset/query-entities-inner-join.md diff --git a/.changeset/query-entities-inner-join.md b/.changeset/query-entities-inner-join.md new file mode 100644 index 0000000000..ee4eb30863 --- /dev/null +++ b/.changeset/query-entities-inner-join.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +**BREAKING**: When paginating entities with an order field via `/entities/by-query`, entities that lack the order field are now excluded from both the result set and the `totalItems` count. Previously these entities appeared at the end of the sorted result via `NULLS LAST`, but cursor-based pagination could not actually reach them past the first page — the count over-reported the number of navigable entities. The new behavior aligns the count with what is actually returned. + +This also removes the `DISTINCT` deduplication from the sort-field CTE, which is a prerequisite for the planner to use the `(key, value, entity_id)` index in sort order and short-circuit on `LIMIT`. Installations with duplicate search rows should land the search-table deduplication migration before adopting this change. diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts index 427a9c8ce0..241af26541 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts @@ -1899,27 +1899,28 @@ describe('DefaultEntitiesCatalog', () => { stitcher, }); - await expect( - catalog - .queryEntities({ - orderFields: [{ field: 'metadata.title', order: 'asc' }], - credentials: mockCredentials.none(), - }) - .then(r => - entitiesResponseToObjects(r.items).map(e => e!.metadata.name), - ), - ).resolves.toEqual(['CC', 'BB', 'AA']); // 'AA' has no title, ends up last + // Entities without the sort field are excluded — sorting by a field + // means "show me entities that have this field, in order." The count + // also reflects only the entities that will be returned. + const ascResult = await catalog.queryEntities({ + orderFields: [{ field: 'metadata.title', order: 'asc' }], + credentials: mockCredentials.none(), + }); + expect( + entitiesResponseToObjects(ascResult.items).map(e => e!.metadata.name), + ).toEqual(['CC', 'BB']); + expect(ascResult.totalItems).toBe(2); - await expect( - catalog - .queryEntities({ - orderFields: [{ field: 'metadata.title', order: 'desc' }], - credentials: mockCredentials.none(), - }) - .then(r => - entitiesResponseToObjects(r.items).map(e => e!.metadata.name), - ), - ).resolves.toEqual(['BB', 'CC', 'AA']); // 'AA' has no title, ends up last + const descResult = await catalog.queryEntities({ + orderFields: [{ field: 'metadata.title', order: 'desc' }], + credentials: mockCredentials.none(), + }); + expect( + entitiesResponseToObjects(descResult.items).map( + e => e!.metadata.name, + ), + ).toEqual(['BB', 'CC']); + expect(descResult.totalItems).toBe(2); }, ); diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index b5a65d6d05..35374cfd15 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -273,33 +273,39 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { const sortField = cursor.orderFields.at(0); // The first part of the query builder is a subquery that applies all of the - // filtering. + // filtering. When a sort field is specified, the search table for that key + // drives the query via INNER JOIN so that the (key, value, entity_id) + // index walks rows in sort order, letting LIMIT short-circuit. Entities + // that lack the sort field are excluded from both the result set and the + // count — this is a deliberate choice that aligns totalItems with the + // number of entities actually reachable through cursor pagination. const dbQuery = this.database.with( 'filtered', ['entity_id', 'final_entity', ...(sortField ? ['value'] : [])], inner => { - inner - .from('final_entities') - .whereNotNull('final_entity'); - if (sortField) { inner - .distinct() - .leftOuterJoin('search', qb => - qb - .on('search.entity_id', 'final_entities.entity_id') - .andOnVal('search.key', sortField.field), + .from('search') + .innerJoin( + 'final_entities', + 'final_entities.entity_id', + 'search.entity_id', ) + .where('search.key', sortField.field) + .whereNotNull('final_entities.final_entity') .select({ entity_id: 'final_entities.entity_id', final_entity: 'final_entities.final_entity', value: 'search.value', }); } else { - inner.select({ - entity_id: 'final_entities.entity_id', - final_entity: 'final_entities.final_entity', - }); + inner + .from('final_entities') + .whereNotNull('final_entity') + .select({ + entity_id: 'final_entities.entity_id', + final_entity: 'final_entities.final_entity', + }); } // Add regular filters and/or predicate query, if given