Skip to content

Commit 6495bfe

Browse files
committed
Do not emit flex search order if it matches primary sort
We thought you should remove the sorting clause to get better performance, but actually sorting is no longer guaranteed if you omit it, and the docs say that performance is improved when sorting matches the primary sort.
1 parent f28f587 commit 6495bfe

File tree

1 file changed

+12
-22
lines changed

1 file changed

+12
-22
lines changed

src/schema-generation/order-by-and-pagination-augmentation.ts

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -119,33 +119,24 @@ export class OrderByAndPaginationAugmentation {
119119
// flexsearch?
120120
// we only offer cursor-based pagination if we can cover all required filters with flex search filters
121121
// this means we just replace the flexsearch listNode with a flexsearch listNode that does the cursor-filtering
122-
let leaveUnordered = false;
123122
if (listNode instanceof FlexSearchQueryNode) {
124123
if (!orderByType) {
125124
throw new Error(`OrderBy type missing for flex search`);
126125
}
127-
// whenever possible, use primary sort
128-
if (orderArgMatchesPrimarySort(args[ORDER_BY_ARG], listNode.rootEntityType.flexSearchPrimarySort)) {
129-
// this would be cleaner if the primary sort was actually parsed into a ModelComponent (see e.g. the Index and IndexField classes)
130-
orderByValues = listNode.rootEntityType.flexSearchPrimarySort.map(clause =>
131-
orderByType.getValueOrThrow(
132-
clause.field.path.replace('.', '_') +
133-
(clause.direction === OrderDirection.ASCENDING ? '_ASC' : '_DESC')
134-
)
135-
);
136-
leaveUnordered = true;
137-
} else {
138-
// this will generate a SORT clause that's not covered by the flexsearch index,
139-
// but the TooManyObject check of flex-search-generator already handles this case to throw
140-
// a TooManyObjects error if needed.
141-
orderByValues = getOrderByValues(args, orderByType, { isAbsoluteOrderRequired });
142-
}
126+
// we used to remove the sort clause if it matched the primary sort, but it turned out this is not
127+
// ok. If the sorting matches the primary sort, sorting is efficient, but you still need to specify
128+
// it, otherwise, sometimes the order can be wrong (after inserting or updating documents).
129+
130+
// this may generate a SORT clause that is not covered by the flexsearch index,
131+
// but the TooManyObject check of flex-search-generator already handles this case to throw
132+
// a TooManyObjects error if needed.
133+
orderByValues = getOrderByValues(args, orderByType, { isAbsoluteOrderRequired });
143134

144135
// for now, cursor-based pagination is only allowed when we can use flexsearch filters for the
145136
// paginationFilter. This simplifies the implementation, but maybe we should support it in the
146137
// future for consistency. Then however we need to adjust the too-many-objects check
147138
// do this check also if cursor is just requested so we get this error on the first page
148-
if (isCursorRequested || !!afterArg) {
139+
if (isCursorRequested || afterArg) {
149140
const violatingClauses = orderByValues.filter(val =>
150141
val.path.some(field => !field.isFlexSearchIndexed)
151142
);
@@ -189,10 +180,9 @@ export class OrderByAndPaginationAugmentation {
189180
}
190181

191182
// sorting always happens on the TransformListQueryNode and not in the FlexSearchQueryNode
192-
const orderBy =
193-
!orderByType || leaveUnordered
194-
? OrderSpecification.UNORDERED
195-
: new OrderSpecification(orderByValues.map(value => value.getClause(objectNode)));
183+
const orderBy = !orderByType
184+
? OrderSpecification.UNORDERED
185+
: new OrderSpecification(orderByValues.map(value => value.getClause(objectNode)));
196186

197187
if (orderBy.isUnordered() && maxCount == undefined && paginationFilter === ConstBoolQueryNode.TRUE) {
198188
return originalListNode;

0 commit comments

Comments
 (0)