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