Unnecessary full-table scan in AggregateCompanies: record fetch runs when only totalCount needed
https://sonarly.com/issue/14733?type=bug The `AggregateCompanies` GraphQL query requests only `totalCount`, but the server unconditionally executes a record-fetching query (`SELECT id ... LIMIT 201`) taking 1730ms, in addition to the efficient 44ms `COUNT(*)` query that actually provides the result. Fix: Skip the expensive record-fetching `queryBuilder.getMany()` call when the GraphQL query only requests aggregate fields (like `totalCount`) and no record fields. The `AggregateCompanies` query requests only `totalCount`, which is parsed into `selectedFieldsResult.aggregate`. The `selectedFieldsResult.select` and `selectedFieldsResult.relations` are both empty. Despite this, the code unconditionally ran `queryBuilder.getMany()` fetching up to 201 records — a 1730ms full-table scan on large workspaces — then discarded the results. The fix adds a `hasRecordFields` check (mirroring the existing `hasAggregatedFields` pattern from commit `1decd40eea`) that gates the entire record-fetching block. When only aggregate fields are requested: - `buildColumnsToSelect`, `setFindOptions`, `take`, and `getMany` are all skipped - `objectRecords` defaults to `[]` - `getPageInfo` handles empty arrays gracefully (returns null cursors) - The aggregate query (`COUNT`) still runs as before This eliminates the 1730ms unnecessary query for aggregate-only requests while preserving all behavior for normal findMany queries that request record fields.
This commit is contained in:
+34
-26
@@ -152,34 +152,45 @@ export class CommonFindManyQueryRunnerService extends CommonBaseQueryRunnerServi
|
||||
objectMetadataNameSingular: flatObjectMetadata.nameSingular,
|
||||
});
|
||||
|
||||
const hasAggregatedFields =
|
||||
Object.keys(args.selectedFieldsResult.aggregate ?? {}).length > 0;
|
||||
|
||||
const hasRecordFields =
|
||||
Object.keys(args.selectedFieldsResult.select ?? {}).length > 0 ||
|
||||
Object.keys(args.selectedFieldsResult.relations ?? {}).length > 0;
|
||||
|
||||
const limit = args.first ?? args.last ?? QUERY_MAX_RECORDS;
|
||||
|
||||
const columnsToSelect = buildColumnsToSelect({
|
||||
select: args.selectedFieldsResult.select,
|
||||
relations: args.selectedFieldsResult.relations,
|
||||
flatObjectMetadata,
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
});
|
||||
let objectRecords: ObjectRecord[] = [];
|
||||
|
||||
if (isDefined(args.offset)) {
|
||||
queryBuilder.skip(args.offset);
|
||||
if (hasRecordFields) {
|
||||
const columnsToSelect = buildColumnsToSelect({
|
||||
select: args.selectedFieldsResult.select,
|
||||
relations: args.selectedFieldsResult.relations,
|
||||
flatObjectMetadata,
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
});
|
||||
|
||||
if (isDefined(args.offset)) {
|
||||
queryBuilder.skip(args.offset);
|
||||
}
|
||||
|
||||
queryBuilder.setFindOptions({ select: columnsToSelect });
|
||||
queryBuilder.take(limit + 1);
|
||||
|
||||
// Add order columns AFTER setFindOptions (setFindOptions clears addSelect)
|
||||
// Pass columnsToSelect so we only add columns that aren't already selected
|
||||
commonQueryParser.addRelationOrderColumnsToBuilder(
|
||||
queryBuilder,
|
||||
parsedOrderBy,
|
||||
flatObjectMetadata.nameSingular,
|
||||
columnsToSelect,
|
||||
);
|
||||
|
||||
objectRecords = (await queryBuilder.getMany()) as ObjectRecord[];
|
||||
}
|
||||
|
||||
queryBuilder.setFindOptions({ select: columnsToSelect });
|
||||
queryBuilder.take(limit + 1);
|
||||
|
||||
// Add order columns AFTER setFindOptions (setFindOptions clears addSelect)
|
||||
// Pass columnsToSelect so we only add columns that aren't already selected
|
||||
commonQueryParser.addRelationOrderColumnsToBuilder(
|
||||
queryBuilder,
|
||||
parsedOrderBy,
|
||||
flatObjectMetadata.nameSingular,
|
||||
columnsToSelect,
|
||||
);
|
||||
|
||||
const objectRecords = (await queryBuilder.getMany()) as ObjectRecord[];
|
||||
|
||||
const pageInfo = getPageInfo(
|
||||
objectRecords,
|
||||
orderByWithIdCondition,
|
||||
@@ -191,9 +202,6 @@ export class CommonFindManyQueryRunnerService extends CommonBaseQueryRunnerServi
|
||||
objectRecords.reverse();
|
||||
}
|
||||
|
||||
const hasAggregatedFields =
|
||||
Object.keys(args.selectedFieldsResult.aggregate ?? {}).length > 0;
|
||||
|
||||
const parentObjectRecordsAggregatedValues = hasAggregatedFields
|
||||
? await aggregateQueryBuilder.getRawOne()
|
||||
: undefined;
|
||||
|
||||
Reference in New Issue
Block a user