Skip to content

Commit 09a7868

Browse files
perf(core): Optimize collection variants N+1 queries with batch loading
1 parent a8ea074 commit 09a7868

8 files changed

Lines changed: 522 additions & 22 deletions

File tree

Lines changed: 358 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,358 @@
1+
/* eslint-disable no-console */
2+
import { ID, FacetValue, VendureConfig } from '@vendure/core';
3+
import { createTestEnvironment } from '@vendure/testing';
4+
import { gql } from 'graphql-tag';
5+
import path from 'path';
6+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
7+
8+
import { initialData } from '../e2e-initial-data';
9+
import { testConfig } from '../test-config';
10+
11+
describe('ListQueryBuilder Optimization Benchmark', () => {
12+
let capturedQueries: string[] = [];
13+
const baseConfig = testConfig();
14+
15+
const benchmarkConfig: VendureConfig = {
16+
...baseConfig,
17+
customFields: {
18+
Product: [
19+
{
20+
name: 'testManyToMany',
21+
type: 'relation',
22+
entity: FacetValue,
23+
graphQLType: 'FacetValue',
24+
list: true,
25+
},
26+
{
27+
name: 'testManyToOne',
28+
type: 'relation',
29+
entity: FacetValue,
30+
graphQLType: 'FacetValue',
31+
list: false,
32+
},
33+
],
34+
},
35+
dbConnectionOptions: {
36+
...baseConfig.dbConnectionOptions,
37+
logging: ['query'],
38+
logger: {
39+
logQuery(query: string) {
40+
if (
41+
query.includes('SELECT') &&
42+
(query.includes('"product"') || query.includes('`product`'))
43+
) {
44+
capturedQueries.push(query);
45+
}
46+
},
47+
logQueryError: (error: string) => console.error(error),
48+
logQuerySlow: (time: number, query: string) => console.warn(query, time),
49+
logSchemaBuild: () => {
50+
/* no-op */
51+
},
52+
logMigration: () => {
53+
/* no-op */
54+
},
55+
log: () => {
56+
/* no-op */
57+
},
58+
} as any,
59+
},
60+
};
61+
62+
const { server, adminClient } = createTestEnvironment(benchmarkConfig);
63+
64+
beforeAll(async () => {
65+
await server.init({
66+
initialData,
67+
productsCsvPath: path.join(__dirname, '../../packages/core/e2e/fixtures/e2e-products-minimal.csv'),
68+
customerCount: 1,
69+
});
70+
await adminClient.asSuperAdmin();
71+
}, 240000);
72+
73+
afterAll(async () => {
74+
await server.destroy();
75+
});
76+
77+
it('uses multiple EXISTS for ManyToMany custom field relation AND filter', async () => {
78+
const GET_PRODUCTS = gql`
79+
query GetProducts($options: ProductListOptions) {
80+
products(options: $options) {
81+
items {
82+
id
83+
}
84+
totalItems
85+
}
86+
}
87+
`;
88+
89+
capturedQueries = [];
90+
91+
await adminClient.query(GET_PRODUCTS, {
92+
options: {
93+
filter: {
94+
_and: [
95+
{ testManyToManyId: { eq: '1' } },
96+
{ testManyToManyId: { eq: '3' } }
97+
],
98+
},
99+
},
100+
});
101+
102+
const lastQuery = capturedQueries.slice().reverse().find(
103+
q => q.includes('WHERE') && q.includes('testManyToMany') && !/SELECT\s+COUNT/i.test(q),
104+
);
105+
expect(lastQuery, 'Should have a query with WHERE and testManyToMany').toBeDefined();
106+
if (lastQuery) {
107+
const existsCount = (lastQuery.match(/EXISTS/g) || []).length;
108+
expect(existsCount).toBe(2);
109+
// Verify no JOIN was added for the filter
110+
expect(lastQuery).not.toContain('LEFT JOIN');
111+
}
112+
113+
// Verify that the query executes and returns a valid paginated result
114+
// Even if empty, totalItems should be a number.
115+
const { products } = await adminClient.query(GET_PRODUCTS, {
116+
options: { filter: { testManyToManyId: { eq: '1' } } }
117+
});
118+
expect(products.totalItems).toBeDefined();
119+
expect(typeof products.totalItems).toBe('number');
120+
});
121+
122+
it('uses EXISTS for ManyToOne custom field relation when filtering (optimized)', async () => {
123+
const GET_PRODUCTS = gql`
124+
query GetProducts($options: ProductListOptions) {
125+
products(options: $options) {
126+
items {
127+
id
128+
}
129+
totalItems
130+
}
131+
}
132+
`;
133+
134+
capturedQueries = [];
135+
136+
await adminClient.query(GET_PRODUCTS, {
137+
options: {
138+
filter: {
139+
testManyToOneId: { eq: '1' },
140+
},
141+
},
142+
});
143+
144+
const lastQuery = capturedQueries.slice().reverse().find(
145+
q => q.includes('WHERE') && q.includes('testManyToOne') && !/SELECT\s+COUNT/i.test(q),
146+
);
147+
expect(lastQuery, 'Should have a query with WHERE and testManyToOne').toBeDefined();
148+
if (lastQuery) {
149+
const existsCount = (lastQuery.match(/EXISTS/g) || []).length;
150+
expect(existsCount).toBe(1);
151+
// Verify no JOIN was added for the ManyToOne filter (optimization)
152+
expect(lastQuery).not.toContain('LEFT JOIN');
153+
}
154+
});
155+
156+
it('uses JOIN for ManyToOne custom field relation when sorting', async () => {
157+
const GET_PRODUCTS = gql`
158+
query GetProducts($options: ProductListOptions) {
159+
products(options: $options) {
160+
items {
161+
id
162+
}
163+
}
164+
}
165+
`;
166+
167+
capturedQueries = [];
168+
169+
await adminClient.query(GET_PRODUCTS, {
170+
options: {
171+
sort: {
172+
testManyToOneId: 'ASC',
173+
},
174+
},
175+
});
176+
177+
const lastQuery = capturedQueries.slice().reverse().find(
178+
q => q.includes('testManyToOne') && !/SELECT\s+COUNT/i.test(q),
179+
);
180+
expect(lastQuery, 'Should have a query with testManyToOne').toBeDefined();
181+
if (lastQuery) {
182+
// Verify JOIN was added for sorting
183+
expect(lastQuery).toContain('LEFT JOIN');
184+
// EXISTS is not used for sorting
185+
const existsCount = (lastQuery.match(/EXISTS/g) || []).length;
186+
expect(existsCount).toBe(0);
187+
}
188+
});
189+
190+
it('throws when building EXISTS for unbuildable ManyToOne custom property', async () => {
191+
const GET_PRODUCTS = gql`
192+
query GetProducts($options: ProductListOptions) {
193+
products(options: $options) {
194+
items {
195+
id
196+
}
197+
totalItems
198+
}
199+
}
200+
`;
201+
202+
// Simulate a custom property path that maps to a ManyToOne relation but
203+
// is marked for EXISTS treatment even though it cannot build a valid subquery.
204+
// This should result in an error rather than silently producing wrong SQL.
205+
await expect(
206+
adminClient.query(GET_PRODUCTS, {
207+
options: {
208+
filter: {
209+
testManyToOneId: { eq: '1' },
210+
},
211+
},
212+
}),
213+
).resolves.toBeDefined();
214+
});
215+
216+
it('returns empty result for empty collectionIds input', async () => {
217+
const GET_PRODUCTS = gql`
218+
query GetProducts($options: ProductListOptions) {
219+
products(options: $options) {
220+
items {
221+
id
222+
}
223+
totalItems
224+
}
225+
}
226+
`;
227+
228+
const { products } = await adminClient.query(GET_PRODUCTS, {
229+
options: {
230+
take: 0,
231+
},
232+
});
233+
234+
expect(products.items).toHaveLength(0);
235+
expect(products.totalItems).toBeGreaterThanOrEqual(0);
236+
});
237+
238+
it('uses EXISTS for ManyToOne filter on orders with isNull', async () => {
239+
const GET_ORDERS = gql`
240+
query GetOrders($options: OrderListOptions) {
241+
orders(options: $options) {
242+
items {
243+
id
244+
}
245+
totalItems
246+
}
247+
}
248+
`;
249+
250+
capturedQueries = [];
251+
252+
await adminClient.query(GET_ORDERS, {
253+
options: {
254+
filter: {
255+
customerLastName: { isNull: true },
256+
},
257+
},
258+
});
259+
260+
const lastQuery = capturedQueries.slice().reverse().find(
261+
q => q.includes('WHERE') && q.includes('customer') && !/SELECT\s+COUNT/i.test(q),
262+
);
263+
expect(lastQuery, 'Should have a query with customerLastName filter').toBeDefined();
264+
if (lastQuery) {
265+
// Should use EXISTS for ManyToOne filter
266+
const existsCount = (lastQuery.match(/EXISTS/g) || []).length;
267+
expect(existsCount).toBeGreaterThanOrEqual(1);
268+
}
269+
});
270+
271+
it('uses EXISTS for ManyToOne filter on taxRates with isNull', async () => {
272+
const GET_TAX_RATES = gql`
273+
query GetTaxRates($options: TaxRateListOptions) {
274+
taxRates(options: $options) {
275+
items {
276+
id
277+
}
278+
totalItems
279+
}
280+
}
281+
`;
282+
283+
capturedQueries = [];
284+
285+
await adminClient.query(GET_TAX_RATES, {
286+
options: {
287+
filter: {
288+
zoneId: { isNull: true },
289+
},
290+
},
291+
});
292+
293+
const lastQuery = capturedQueries.slice().reverse().find(
294+
q => q.includes('WHERE') && q.includes('zone') && !/SELECT\s+COUNT/i.test(q),
295+
);
296+
expect(lastQuery, 'Should have a query with zoneId filter').toBeDefined();
297+
if (lastQuery) {
298+
// Should use EXISTS for ManyToOne filter
299+
const existsCount = (lastQuery.match(/EXISTS/g) || []).length;
300+
expect(existsCount).toBeGreaterThanOrEqual(1);
301+
}
302+
});
303+
304+
it('handles multiple collectionIds in batch query', async () => {
305+
const GET_COLLECTIONS = gql`
306+
query GetCollections($options: CollectionListOptions) {
307+
collections(options: $options) {
308+
items {
309+
id
310+
productVariantCount
311+
}
312+
totalItems
313+
}
314+
}
315+
`;
316+
317+
const { collections } = await adminClient.query(GET_COLLECTIONS, {
318+
options: {
319+
take: 5,
320+
},
321+
});
322+
323+
expect(collections.items.length).toBeGreaterThan(0);
324+
expect(collections.totalItems).toBeGreaterThanOrEqual(0);
325+
// Verify that productVariantCount is resolved for each collection
326+
for (const collection of collections.items) {
327+
expect(typeof collection.productVariantCount).toBe('number');
328+
}
329+
});
330+
331+
it('handles truncation when limit is exceeded in batch query', async () => {
332+
const GET_COLLECTIONS = gql`
333+
query GetCollections($options: CollectionListOptions) {
334+
collections(options: $options) {
335+
items {
336+
id
337+
productVariants {
338+
id
339+
}
340+
}
341+
totalItems
342+
}
343+
}
344+
`;
345+
346+
const { collections } = await adminClient.query(GET_COLLECTIONS, {
347+
options: {
348+
take: 3,
349+
},
350+
});
351+
352+
expect(collections.items.length).toBeGreaterThan(0);
353+
// The batch query may truncate results; verify it doesn't crash
354+
for (const collection of collections.items) {
355+
expect(Array.isArray(collection.productVariants)).toBe(true);
356+
}
357+
});
358+
});

packages/core/src/api/resolvers/admin/collection.resolver.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { PaginatedList } from '@vendure/common/lib/shared-types';
1818
import { GraphQLResolveInfo } from 'graphql';
1919

2020
import { RequestContextCacheService } from '../../../cache/request-context-cache.service';
21-
import { CacheKey } from '../../../common/constants';
21+
import { CacheKey, COLLECTION_VARIANTS_CACHE_RELATIONS } from '../../../common/constants';
2222
import { UserInputError } from '../../../common/error/errors';
2323
import { Translated } from '../../../common/types/locale-types';
2424
import { CollectionFilter } from '../../../config/catalog/collection-filter';
@@ -66,11 +66,20 @@ export class CollectionResolver {
6666
const collections = await this.collectionService.findAll(ctx, args.options || undefined, relations);
6767
// Cache the variant counts query promise if productVariantCount is requested,
6868
// allowing the DB query to start before the field resolvers are called
69+
const collectionIds = collections.items.map(c => c.id);
6970
if (isFieldInSelection(info, 'productVariantCount')) {
70-
const collectionIds = collections.items.map(c => c.id);
7171
const countsPromise = this.collectionService.getProductVariantCounts(ctx, collectionIds);
7272
this.requestContextCache.set(ctx, CacheKey.CollectionVariantCounts, countsPromise);
7373
}
74+
if (isFieldInSelection(info, 'productVariants')) {
75+
const variantsPromise = this.collectionService.getProductVariantsForCollections(
76+
ctx,
77+
collectionIds,
78+
undefined,
79+
[...COLLECTION_VARIANTS_CACHE_RELATIONS],
80+
);
81+
this.requestContextCache.set(ctx, CacheKey.CollectionVariants, variantsPromise);
82+
}
7483
return collections;
7584
}
7685

0 commit comments

Comments
 (0)