diff --git a/.changeset/entities-order-driver.md b/.changeset/entities-order-driver.md new file mode 100644 index 0000000000..a2ad0e478b --- /dev/null +++ b/.changeset/entities-order-driver.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Restructured the entity listing endpoint so that, when a sort field is specified, the search-by-key index drives the query rather than being side-joined onto `final_entities`. This lets PostgreSQL walk the `(key, value, entity_id)` index in already-sorted order and short-circuit on `LIMIT`, reducing typical broad-filter paginated list times from seconds to milliseconds. Entities that lack the sort field still appear at the end of sorted results (NULLS LAST semantics preserved), ordered by `entity_id`. diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts index 5d34ebe3f1..80637e6bae 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts @@ -741,6 +741,185 @@ describe('DefaultEntitiesCatalog', () => { ).resolves.toEqual(['n4', 'n3', 'n1', 'n2']); }, ); + + it.each(databases.eachSupportedId())( + 'paginates correctly through single-field ordering, %p', + async databaseId => { + await createDatabase(databaseId); + + // All four entities have metadata.name — fast path uses Phase 1 only + for (const name of ['n1', 'n2', 'n3', 'n4']) { + await addEntityToSearch({ + apiVersion: 'a', + kind: 'k', + metadata: { name }, + }); + } + + const catalog = new DefaultEntitiesCatalog({ + database: knex, + logger: mockServices.logger.mock(), + stitcher, + }); + + async function page(limit: number, offset?: number): Promise { + const r = await catalog.entities({ + order: [{ field: 'metadata.name', order: 'asc' }], + pagination: { limit, offset }, + credentials: mockCredentials.none(), + }); + return entitiesResponseToObjects(r.entities).map( + e => e!.metadata.name, + ); + } + + async function hasNext( + limit: number, + offset?: number, + ): Promise { + const r = await catalog.entities({ + order: [{ field: 'metadata.name', order: 'asc' }], + pagination: { limit, offset }, + credentials: mockCredentials.none(), + }); + return r.pageInfo.hasNextPage; + } + + await expect(page(2)).resolves.toEqual(['n1', 'n2']); + expect(await hasNext(2)).toBe(true); + + await expect(page(2, 2)).resolves.toEqual(['n3', 'n4']); + expect(await hasNext(2, 2)).toBe(false); + + await expect(page(2, 1)).resolves.toEqual(['n2', 'n3']); + expect(await hasNext(2, 1)).toBe(true); + + await expect(page(100)).resolves.toEqual(['n1', 'n2', 'n3', 'n4']); + }, + ); + + it.each(databases.eachSupportedId())( + 'paginates across the Phase 1 / Phase 2 boundary, %p', + async databaseId => { + await createDatabase(databaseId); + + // n1 and n2 have spec.b (Phase 1); n3 and n4 do not (Phase 2). + // Explicit UIDs pin Phase 2 ordering (entity_id ASC) to a known sequence. + await addEntityToSearch({ + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n1' }, + spec: { b: 'alpha' }, + }); + await addEntityToSearch({ + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n2' }, + spec: { b: 'beta' }, + }); + await addEntityToSearch({ + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n3', uid: 'aaaa-n3' }, + }); + await addEntityToSearch({ + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n4', uid: 'bbbb-n4' }, + }); + + const catalog = new DefaultEntitiesCatalog({ + database: knex, + logger: mockServices.logger.mock(), + stitcher, + }); + + async function page( + limit: number, + offset?: number, + order: 'asc' | 'desc' = 'asc', + ): Promise { + const r = await catalog.entities({ + order: [{ field: 'spec.b', order }], + pagination: { limit, offset }, + credentials: mockCredentials.none(), + }); + return entitiesResponseToObjects(r.entities).map( + e => e!.metadata.name, + ); + } + + // Page that straddles the Phase 1 / Phase 2 boundary + await expect(page(3)).resolves.toEqual(['n1', 'n2', 'n3']); + await expect(page(3, 1)).resolves.toEqual(['n2', 'n3', 'n4']); + + // Phase 2 entities (no spec.b) are always ordered ASC by entity_id + // regardless of the primary sort direction + await expect(page(4, 0, 'asc')).resolves.toEqual([ + 'n1', + 'n2', + 'n3', + 'n4', + ]); + await expect(page(4, 0, 'desc')).resolves.toEqual([ + 'n2', + 'n1', + 'n3', + 'n4', + ]); + }, + ); + + it.each(databases.eachSupportedId())( + 'treats a null sort-field value the same as a missing sort field, %p', + async databaseId => { + await createDatabase(databaseId); + + // n1 has spec.b with a real value (Phase 1) + // n2 has spec.b explicitly set to null — buildEntitySearch stores value=NULL + // n3 has no spec.b at all + // n2 and n3 must both end up in the NULLS-LAST bucket (Phase 2), + // ordered by entity_id, regardless of primary sort direction. + await addEntityToSearch({ + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n1' }, + spec: { b: 'alpha' }, + }); + await addEntityToSearch({ + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n2', uid: 'aaaa-n2' }, + spec: { b: null }, + }); + await addEntityToSearch({ + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n3', uid: 'bbbb-n3' }, + }); + + const catalog = new DefaultEntitiesCatalog({ + database: knex, + logger: mockServices.logger.mock(), + stitcher, + }); + + async function page(order: 'asc' | 'desc'): Promise { + const r = await catalog.entities({ + order: [{ field: 'spec.b', order }], + credentials: mockCredentials.none(), + }); + return entitiesResponseToObjects(r.entities).map( + e => e!.metadata.name, + ); + } + + // n2 (null value) and n3 (missing key) must sort together after n1, + // ordered by entity_id ASC, regardless of primary direction + await expect(page('asc')).resolves.toEqual(['n1', 'n2', 'n3']); + await expect(page('desc')).resolves.toEqual(['n1', 'n2', 'n3']); + }, + ); }); describe('entitiesBatch', () => { diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index 0d896ca6f4..547d276eb8 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -122,64 +122,89 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { async entities(request?: EntitiesRequest): Promise { const db = this.database; const { limit, offset } = parsePagination(request?.pagination); + const primaryOrder = request?.order?.[0]; - let entitiesQuery = - db('final_entities').select('final_entities.*'); - - request?.order?.forEach(({ field }, index) => { - const alias = `order_${index}`; - entitiesQuery = entitiesQuery.leftOuterJoin( - { [alias]: 'search' }, - function search(inner) { - inner - .on(`${alias}.entity_id`, 'final_entities.entity_id') - .andOn(`${alias}.key`, db.raw('?', [field])); - }, + // When exactly one order field is specified we run a two-phase fetch + // that drives from the search-by-key index for that field. The index + // walks rows in already-sorted order, so the planner can short-circuit + // on LIMIT instead of having to materialise and sort the full filtered + // set. Phase 2 appends entities that lack the order field (NULLS LAST) + // and is skipped when phase 1 already fills the request. + // + // Multi-field ordering falls back to the original LEFT JOIN shape + // because tie-breaking on a second field requires materialisation of + // the full set anyway. + const useFastPath = primaryOrder && (request?.order?.length ?? 0) <= 1; + let rows: DbFinalEntitiesRow[]; + if (useFastPath) { + rows = await this.runOrderedEntitiesQuery( + request!, + primaryOrder, + limit, + offset, ); - }); - - entitiesQuery = entitiesQuery.whereNotNull('final_entities.final_entity'); - - if (request?.filter) { - entitiesQuery = applyEntityFilterToQuery({ - filter: request.filter, - targetQuery: entitiesQuery, - onEntityIdField: 'final_entities.entity_id', - knex: db, - }); - } - - request?.order?.forEach(({ order }, index) => { - if (db.client.config.client === 'pg') { - // pg correctly orders by the column value and handling nulls in one go - entitiesQuery = entitiesQuery.orderBy([ - { column: `order_${index}.value`, order, nulls: 'last' }, - ]); - } else { - // sqlite and mysql translate the above statement ONLY into "order by (value is null) asc" - // no matter what the order is, for some reason, so we have to manually add back the statement - // that translates to "order by value " while avoiding to give an order - entitiesQuery = entitiesQuery.orderBy([ - { column: `order_${index}.value`, order: undefined, nulls: 'last' }, - { column: `order_${index}.value`, order }, - ]); - } - }); - - if (!request?.order) { - entitiesQuery = entitiesQuery.orderBy('final_entities.entity_ref', 'asc'); // default sort } else { - entitiesQuery.orderBy('final_entities.entity_id', 'asc'); // stable sort + let entitiesQuery = + db('final_entities').select('final_entities.*'); + + request?.order?.forEach(({ field }, index) => { + const alias = `order_${index}`; + entitiesQuery = entitiesQuery.leftOuterJoin( + { [alias]: 'search' }, + function search(inner) { + inner + .on(`${alias}.entity_id`, 'final_entities.entity_id') + .andOn(`${alias}.key`, db.raw('?', [field])); + }, + ); + }); + + entitiesQuery = entitiesQuery.whereNotNull('final_entities.final_entity'); + + if (request?.filter) { + entitiesQuery = applyEntityFilterToQuery({ + filter: request.filter, + targetQuery: entitiesQuery, + onEntityIdField: 'final_entities.entity_id', + knex: db, + }); + } + + if (request?.order) { + request.order.forEach(({ order }, index) => { + if (db.client.config.client === 'pg') { + entitiesQuery = entitiesQuery.orderBy([ + { column: `order_${index}.value`, order, nulls: 'last' }, + ]); + } else { + entitiesQuery = entitiesQuery.orderBy([ + { + column: `order_${index}.value`, + order: undefined, + nulls: 'last', + }, + { column: `order_${index}.value`, order }, + ]); + } + }); + entitiesQuery.orderBy('final_entities.entity_id', 'asc'); + } else { + entitiesQuery = entitiesQuery.orderBy( + 'final_entities.entity_ref', + 'asc', + ); + } + + if (limit !== undefined) { + entitiesQuery = entitiesQuery.limit(limit + 1); + } + if (offset !== undefined) { + entitiesQuery = entitiesQuery.offset(offset); + } + + rows = await entitiesQuery; } - if (limit !== undefined) { - entitiesQuery = entitiesQuery.limit(limit + 1); - } - if (offset !== undefined) { - entitiesQuery = entitiesQuery.offset(offset); - } - - let rows = await entitiesQuery; let pageInfo: DbPageInfo; if (limit === undefined || rows.length <= limit) { pageInfo = { hasNextPage: false }; @@ -211,6 +236,107 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { }; } + /** + * Two-phase fetch used when the caller has specified an order field. + * See entities() for a longer description of the rationale. + */ + private async runOrderedEntitiesQuery( + request: EntitiesRequest, + primaryOrder: EntityOrder, + limit: number | undefined, + offset: number | undefined, + ): Promise { + const db = this.database; + const isPg = db.client.config.client === 'pg'; + const wantedRows = + limit === undefined ? Number.MAX_SAFE_INTEGER : (offset ?? 0) + limit + 1; + + const applyFilter = ( + query: Knex.QueryBuilder, + ): Knex.QueryBuilder => { + if (!request.filter) { + return query; + } + return applyEntityFilterToQuery({ + filter: request.filter, + targetQuery: query, + onEntityIdField: 'final_entities.entity_id', + knex: db, + }); + }; + + // Phase 1 -- entities that have a non-NULL value for the order field. + // Rows where the key exists but value IS NULL (e.g. the entity field is + // explicitly null, or exceeded MAX_VALUE_LENGTH in buildEntitySearch) are + // excluded here so they fall through to Phase 2 and sort in the same + // NULLS-LAST bucket as entities that have no row for the key at all — + // preserving the semantics of the previous LEFT JOIN approach. + let withField = db('search as order_0') + .innerJoin( + 'final_entities', + 'final_entities.entity_id', + 'order_0.entity_id', + ) + .where('order_0.key', primaryOrder.field) + .whereNotNull('order_0.value') + .whereNotNull('final_entities.final_entity') + .select('final_entities.*'); + withField = applyFilter(withField); + withField = isPg + ? withField.orderBy([ + { column: 'order_0.value', order: primaryOrder.order, nulls: 'last' }, + { column: 'final_entities.entity_id', order: 'asc' }, + ]) + : withField.orderBy([ + { column: 'order_0.value', order: undefined, nulls: 'last' }, + { column: 'order_0.value', order: primaryOrder.order }, + { column: 'final_entities.entity_id', order: 'asc' }, + ]); + if (wantedRows < Number.MAX_SAFE_INTEGER) { + withField = withField.limit(wantedRows); + } + const withFieldRows = await withField; + + // If phase 1 already covered everything we asked for, skip the second + // phase entirely. This is the common UI case where every entity in the + // filtered set has the order field. + if (withFieldRows.length >= wantedRows) { + const skip = offset ?? 0; + return withFieldRows.slice(skip, skip + (limit ?? wantedRows) + 1); + } + + // Phase 2 -- entities that lack the order field, appended after. + let withoutField = db('final_entities') + .select('final_entities.*') + .whereNotNull('final_entities.final_entity') + .whereNotExists(qb => + qb + .from('search') + .where('search.entity_id', db.ref('final_entities.entity_id')) + .andWhere('search.key', primaryOrder.field) + .whereNotNull('search.value'), + ); + withoutField = applyFilter(withoutField); + withoutField = withoutField.orderBy( + 'final_entities.entity_id', + 'asc', // NULL group always stable-sorted ASC regardless of primary direction + ); + if (limit !== undefined) { + // Phase 2 only contributes the rows that phase 1 didn't cover. + const remaining = + wantedRows - Math.min(withFieldRows.length, (offset ?? 0) + limit + 1); + withoutField = withoutField.limit(Math.max(0, remaining)); + } + const withoutFieldRows = await withoutField; + + const combined = [...withFieldRows, ...withoutFieldRows]; + if (limit === undefined) { + return combined.slice(offset ?? 0); + } + const skip = offset ?? 0; + return combined.slice(skip, skip + limit + 1); + } + async entitiesBatch( request: EntitiesBatchRequest, ): Promise {