Merge pull request #34175 from backstage/freben/query-entities-inner-join
catalog-backend: drive queryEntities ordering from search-by-key
This commit is contained in:
@@ -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.
|
||||
@@ -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);
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -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<DbFinalEntitiesRow>('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<DbFinalEntitiesRow>('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
|
||||
|
||||
Reference in New Issue
Block a user