diff --git a/e2e-common/benchmarks/benchmark-list-query.e2e-spec.ts b/e2e-common/benchmarks/benchmark-list-query.e2e-spec.ts new file mode 100644 index 0000000000..69aaae1d1b --- /dev/null +++ b/e2e-common/benchmarks/benchmark-list-query.e2e-spec.ts @@ -0,0 +1,189 @@ +/* eslint-disable no-console */ +import { ID, FacetValue, VendureConfig } from '@vendure/core'; +import { createTestEnvironment } from '@vendure/testing'; +import { gql } from 'graphql-tag'; +import path from 'path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { initialData } from '../e2e-initial-data'; +import { testConfig } from '../test-config'; + +describe('ListQueryBuilder Optimization Benchmark', () => { + let capturedQueries: string[] = []; + const baseConfig = testConfig(); + + const benchmarkConfig: VendureConfig = { + ...baseConfig, + customFields: { + Product: [ + { + name: 'testManyToMany', + type: 'relation', + entity: FacetValue, + graphQLType: 'FacetValue', + list: true, + }, + { + name: 'testManyToOne', + type: 'relation', + entity: FacetValue, + graphQLType: 'FacetValue', + list: false, + }, + ], + }, + dbConnectionOptions: { + ...baseConfig.dbConnectionOptions, + logging: ['query'], + logger: { + logQuery(query: string) { + if ( + query.includes('SELECT') && + (query.includes('"product"') || query.includes('`product`')) + ) { + capturedQueries.push(query); + } + }, + logQueryError: (error: string) => console.error(error), + logQuerySlow: (time: number, query: string) => console.warn(query, time), + logSchemaBuild: () => { + /* no-op */ + }, + logMigration: () => { + /* no-op */ + }, + log: () => { + /* no-op */ + }, + } as any, + }, + }; + + const { server, adminClient } = createTestEnvironment(benchmarkConfig); + + beforeAll(async () => { + await server.init({ + initialData, + productsCsvPath: path.join(__dirname, '../../packages/core/e2e/fixtures/e2e-products-minimal.csv'), + customerCount: 1, + }); + await adminClient.asSuperAdmin(); + }, 240000); + + afterAll(async () => { + await server.destroy(); + }); + + it('uses multiple EXISTS for ManyToMany custom field relation AND filter', async () => { + const GET_PRODUCTS = gql` + query GetProducts($options: ProductListOptions) { + products(options: $options) { + items { + id + } + totalItems + } + } + `; + + capturedQueries = []; + + await adminClient.query(GET_PRODUCTS, { + options: { + filter: { + _and: [ + { testManyToManyId: { eq: '1' } }, + { testManyToManyId: { eq: '3' } } + ], + }, + }, + }); + + const lastQuery = capturedQueries.slice().reverse().find( + q => q.includes('WHERE') && q.includes('testManyToMany') && !/SELECT\s+COUNT/i.test(q), + ); + expect(lastQuery, 'Should have a query with WHERE and testManyToMany').toBeDefined(); + if (lastQuery) { + const existsCount = (lastQuery.match(/EXISTS/g) || []).length; + expect(existsCount).toBe(2); + // Verify no JOIN was added for the filter + expect(lastQuery).not.toContain('LEFT JOIN'); + } + + // Verify that the query executes and returns a valid paginated result + // Even if empty, totalItems should be a number. + const { products } = await adminClient.query(GET_PRODUCTS, { + options: { filter: { testManyToManyId: { eq: '1' } } } + }); + expect(products.totalItems).toBeDefined(); + expect(typeof products.totalItems).toBe('number'); + }); + + it('uses EXISTS for ManyToOne custom field relation when filtering (optimized)', async () => { + const GET_PRODUCTS = gql` + query GetProducts($options: ProductListOptions) { + products(options: $options) { + items { + id + } + totalItems + } + } + `; + + capturedQueries = []; + + await adminClient.query(GET_PRODUCTS, { + options: { + filter: { + testManyToOneId: { eq: '1' }, + }, + }, + }); + + const lastQuery = capturedQueries.slice().reverse().find( + q => q.includes('WHERE') && q.includes('testManyToOne') && !/SELECT\s+COUNT/i.test(q), + ); + expect(lastQuery, 'Should have a query with WHERE and testManyToOne').toBeDefined(); + if (lastQuery) { + const existsCount = (lastQuery.match(/EXISTS/g) || []).length; + expect(existsCount).toBe(1); + // Verify no JOIN was added for the ManyToOne filter (optimization) + expect(lastQuery).not.toContain('LEFT JOIN'); + } + }); + + it('uses JOIN for ManyToOne custom field relation when sorting', async () => { + const GET_PRODUCTS = gql` + query GetProducts($options: ProductListOptions) { + products(options: $options) { + items { + id + } + } + } + `; + + capturedQueries = []; + + await adminClient.query(GET_PRODUCTS, { + options: { + sort: { + testManyToOneId: 'ASC', + }, + }, + }); + + const lastQuery = capturedQueries.slice().reverse().find( + q => q.includes('testManyToOne') && !/SELECT\s+COUNT/i.test(q), + ); + expect(lastQuery, 'Should have a query with testManyToOne').toBeDefined(); + if (lastQuery) { + // Verify JOIN was added for sorting + expect(lastQuery).toContain('LEFT JOIN'); + // EXISTS is not used for sorting + const existsCount = (lastQuery.match(/EXISTS/g) || []).length; + expect(existsCount).toBe(0); + } + }); +}); diff --git a/packages/core/src/api/resolvers/admin/collection.resolver.ts b/packages/core/src/api/resolvers/admin/collection.resolver.ts index 53cca2a040..57e8063e94 100644 --- a/packages/core/src/api/resolvers/admin/collection.resolver.ts +++ b/packages/core/src/api/resolvers/admin/collection.resolver.ts @@ -18,7 +18,7 @@ import { PaginatedList } from '@vendure/common/lib/shared-types'; import { GraphQLResolveInfo } from 'graphql'; import { RequestContextCacheService } from '../../../cache/request-context-cache.service'; -import { CacheKey } from '../../../common/constants'; +import { CacheKey, COLLECTION_VARIANTS_CACHE_RELATIONS } from '../../../common/constants'; import { UserInputError } from '../../../common/error/errors'; import { Translated } from '../../../common/types/locale-types'; import { CollectionFilter } from '../../../config/catalog/collection-filter'; @@ -66,11 +66,20 @@ export class CollectionResolver { const collections = await this.collectionService.findAll(ctx, args.options || undefined, relations); // Cache the variant counts query promise if productVariantCount is requested, // allowing the DB query to start before the field resolvers are called + const collectionIds = collections.items.map(c => c.id); if (isFieldInSelection(info, 'productVariantCount')) { - const collectionIds = collections.items.map(c => c.id); const countsPromise = this.collectionService.getProductVariantCounts(ctx, collectionIds); this.requestContextCache.set(ctx, CacheKey.CollectionVariantCounts, countsPromise); } + if (isFieldInSelection(info, 'productVariants')) { + const variantsPromise = this.collectionService.getProductVariantsForCollections( + ctx, + collectionIds, + undefined, + [...COLLECTION_VARIANTS_CACHE_RELATIONS], + ); + this.requestContextCache.set(ctx, CacheKey.CollectionVariants, variantsPromise); + } return collections; } diff --git a/packages/core/src/api/resolvers/entity/collection-entity.resolver.ts b/packages/core/src/api/resolvers/entity/collection-entity.resolver.ts index f005eab349..3a35b1c600 100644 --- a/packages/core/src/api/resolvers/entity/collection-entity.resolver.ts +++ b/packages/core/src/api/resolvers/entity/collection-entity.resolver.ts @@ -8,10 +8,11 @@ import { import { ID, PaginatedList } from '@vendure/common/lib/shared-types'; import { RequestContextCacheService } from '../../../cache/request-context-cache.service'; -import { CacheKey } from '../../../common/constants'; +import { CacheKey, COLLECTION_VARIANTS_CACHE_RELATIONS } from '../../../common/constants'; import { ListQueryOptions } from '../../../common/types/common-types'; import { Translated } from '../../../common/types/locale-types'; import { CollectionFilter } from '../../../config/catalog/collection-filter'; +import { ConfigService } from '../../../config/config.service'; import { Asset, Collection, Product, ProductVariant } from '../../../entity'; import { LocaleStringHydrator } from '../../../service/helpers/locale-string-hydrator/locale-string-hydrator'; import { AssetService } from '../../../service/services/asset.service'; @@ -33,6 +34,7 @@ export class CollectionEntityResolver { private localeStringHydrator: LocaleStringHydrator, private configurableOperationCodec: ConfigurableOperationCodec, private requestContextCache: RequestContextCacheService, + private configService: ConfigService, ) {} @ResolveField() @@ -63,6 +65,40 @@ export class CollectionEntityResolver { @Api() apiType: ApiType, @Relations({ entity: ProductVariant, omit: ['assets'] }) relations: RelationPaths, ): Promise>> { + const isDefaultOptions = !args.options || Object.keys(args.options).length === 0; + if (isDefaultOptions && apiType === 'admin') { + const cachedVariantsPromise = this.requestContextCache.get< + Promise> + >(ctx, CacheKey.CollectionVariants); + if (cachedVariantsPromise) { + const variantsMap = await cachedVariantsPromise; + const variants = variantsMap.get(String(collection.id)); + if (variants) { + // Check if the requested relations are compatible with the cached data. + // The cache was populated with default relations defined by COLLECTION_VARIANTS_CACHE_RELATIONS. + // We can use the cache ONLY if the requested relations are a subset of or equal to the default relations. + const isCacheCompatible = relations.every(rel => + (COLLECTION_VARIANTS_CACHE_RELATIONS as readonly string[]).includes(rel), + ); + + if (isCacheCompatible) { + // Cache is compatible, use it. + const { adminListQueryLimit } = this.configService.apiOptions; + const skip = args.options?.skip ?? 0; + const take = args.options?.take ?? adminListQueryLimit; + const items = await this.productVariantService.applyPricesAndTranslateVariants( + ctx, + variants.slice(skip, skip + take), + ); + return { + items, + totalItems: variants.length, + }; + } + } + } + } + let options: ListQueryOptions = args.options; if (apiType === 'shop') { options = { diff --git a/packages/core/src/common/constants.ts b/packages/core/src/common/constants.ts index d12bf385b2..6ab36b1f08 100644 --- a/packages/core/src/common/constants.ts +++ b/packages/core/src/common/constants.ts @@ -85,6 +85,12 @@ export const CacheKey = { ActiveTaxZone: (channelId: ID) => `ActiveTaxZone:${channelId}`, ActiveTaxZone_PPA: (channelId: ID) => `ActiveTaxZone_PPA:${channelId}`, CollectionVariantCounts: 'CollectionService.getProductVariantCounts', + CollectionVariants: 'CollectionService.getProductVariantsForCollections', ExhaustedPromotions: (channelId: ID, customerId: ID | undefined) => `ExhaustedPromotions:${channelId}:${customerId ?? 'guest'}`, }; + +/** + * The default relations used when pre-caching product variants for collections. + */ +export const COLLECTION_VARIANTS_CACHE_RELATIONS = ['taxCategory'] as const; diff --git a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts index 35760e310f..1e3b1048f3 100644 --- a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts +++ b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts @@ -401,8 +401,12 @@ export class ListQueryBuilder implements OnApplicationBootstrap { } else { qb.orWhere(existsClause.clause, existsClause.parameters); } - return; + } else { + throw new Error( + `Could not build EXISTS subquery for custom property "${condition.isExistsCondition.customPropertyKey}". This filter condition cannot be applied.`, + ); } + return; } // Standard WHERE clause handling @@ -480,7 +484,10 @@ export class ListQueryBuilder implements OnApplicationBootstrap { // Helper to escape identifiers for the current database driver (handles PostgreSQL quoting) const escapeId = (name: string) => mainQb.connection.driver.escape(name); const escapeTablePath = (path: string) => - path.split('.').map(segment => mainQb.connection.driver.escape(segment)).join('.'); + path + .split('.') + .map(segment => mainQb.connection.driver.escape(segment)) + .join('.'); let existsQuery: string; @@ -553,6 +560,29 @@ export class ListQueryBuilder implements OnApplicationBootstrap { SELECT 1 FROM ${escapeTablePath(inverseTableName)} ${escapeId(relatedAlias)} WHERE ${escapeId(relatedAlias)}.${escapeId(foreignKeyColumn)} = ${escapeId(mainQb.alias)}.${escapeId('id')} AND ${whereCondition} )`; + } else if (relation.isManyToOne) { + // ManyToOne: The foreign key is on the main entity table + const relatedAlias = aliasBase; + const joinColumns = relation.joinColumns; + if (!joinColumns || joinColumns.length === 0) { + return null; + } + const foreignKeyColumn = joinColumns[0].databaseName; + + const whereCondition = this.buildWhereConditionClause( + relatedAlias, + columnName, + comparisonOperator, + newParamKey, + escapeId, + ); + + // EXISTS (SELECT 1 FROM related_table rt + // WHERE rt.id = main_entity.foreignKey AND rt.columnName = :paramValue) + existsQuery = `EXISTS ( + SELECT 1 FROM ${escapeTablePath(inverseTableName)} ${escapeId(relatedAlias)} + WHERE ${escapeId(relatedAlias)}.${escapeId('id')} = ${escapeId(mainQb.alias)}.${escapeId(foreignKeyColumn)} AND ${whereCondition} + )`; } else { // Not a *-to-Many relation, shouldn't happen but fall back gracefully return null; @@ -672,7 +702,18 @@ export class ListQueryBuilder implements OnApplicationBootstrap { // to join the associated relations. continue; } - const relationPath = path.split('.').slice(0, -1); + const parts = path.split('.'); + const relationPath = parts.slice(0, -1); + + // Optimization: If the custom property is a ManyToOne relation and is NOT being used for sorting, + // we can skip the JOIN and let the filter be handled by an EXISTS subquery. + if (relationPath.length === 1) { + const relationMetadata = metadata.findRelationWithPropertyPath(relationPath[0]); + if (relationMetadata?.isManyToOne && !(options.sort as any)?.[property]) { + continue; + } + } + let targetMetadata = metadata; const reconstructedPath = []; for (const relationPathPart of relationPath) { @@ -689,7 +730,7 @@ export class ListQueryBuilder implements OnApplicationBootstrap { } private customPropertyIsBeingUsed(property: string, options: ListQueryOptions): boolean { - return !!(options.sort?.[property] || this.isPropertyUsedInFilter(property, options.filter)); + return !!((options.sort as any)?.[property] || this.isPropertyUsedInFilter(property, options.filter)); } private isPropertyUsedInFilter( @@ -720,6 +761,19 @@ export class ListQueryBuilder implements OnApplicationBootstrap { continue; } let parts = customPropertyMap[property].split('.'); + + // Optimization: If the custom property is a ManyToOne relation and is NOT being used for sorting, + // we can skip the JOIN and let the filter be handled by an EXISTS subquery. + // This avoids performance issues when many custom fields are present. + if (parts.length === 2) { + const relationMetadata = qb.expressionMap.mainAlias?.metadata.findRelationWithPropertyPath( + parts[0], + ); + if (relationMetadata?.isManyToOne && !(options.sort as any)?.[property]) { + continue; + } + } + const normalizedRelationPath: string[] = []; let entityMetadata = qb.expressionMap.mainAlias?.metadata; let entityAlias = qb.alias; diff --git a/packages/core/src/service/helpers/list-query-builder/parse-filter-params.ts b/packages/core/src/service/helpers/list-query-builder/parse-filter-params.ts index 250c2293f9..6219a43c5c 100644 --- a/packages/core/src/service/helpers/list-query-builder/parse-filter-params.ts +++ b/packages/core/src/service/helpers/list-query-builder/parse-filter-params.ts @@ -214,7 +214,10 @@ function getToManyRelationCustomProperties( const relationName = pathParts[0]; const relationMetadata = metadata.findRelationWithPropertyPath(relationName); - if (relationMetadata && (relationMetadata.isOneToMany || relationMetadata.isManyToMany)) { + if ( + relationMetadata && + (relationMetadata.isOneToMany || relationMetadata.isManyToMany || relationMetadata.isManyToOne) + ) { toManyProperties.add(property); } } diff --git a/packages/core/src/service/services/collection.service.ts b/packages/core/src/service/services/collection.service.ts index 90705e2eb1..75af451d01 100644 --- a/packages/core/src/service/services/collection.service.ts +++ b/packages/core/src/service/services/collection.service.ts @@ -490,7 +490,7 @@ export class CollectionService implements OnModuleInit { ctx: RequestContext, input: PreviewCollectionVariantsInput, options?: ListQueryOptions, - relations?: RelationPaths, + relations?: RelationPaths, ): Promise> { const applicableFilters = this.getCollectionFiltersFromInput(input); if (input.parentId && input.inheritFilters) { @@ -503,13 +503,18 @@ export class CollectionService implements OnModuleInit { ); applicableFilters.push(...parentFilters, ...ancestorFilters); } - let qb = this.listQueryBuilder.build(ProductVariant, options, { - relations: relations ?? ['taxCategory'], - channelId: ctx.channelId, - where: { deletedAt: IsNull() }, - ctx, - entityAlias: 'productVariant', - }); + + let qb = this.listQueryBuilder.build( + ProductVariant, + options || { take: undefined, skip: undefined }, + { + relations: relations ?? ['taxCategory'], + channelId: ctx.channelId, + where: { deletedAt: IsNull() }, + ctx, + entityAlias: 'productVariant', + }, + ); const { collectionFilters } = this.configService.catalogOptions; for (const filterType of collectionFilters) { @@ -1041,4 +1046,74 @@ export class CollectionService implements OnModuleInit { ) .then(collections => collections.map(collection => this.translator.translate(collection, ctx))); } + + /** + * @description + * Returns a Map of collection IDs to their associated product variants. + * This performs a single bulk query to get all variants for all provided collection IDs, + * avoiding N+1 query issues when resolving variants on multiple collections. + */ + async getProductVariantsForCollections( + ctx: RequestContext, + collectionIds: ID[], + options?: ListQueryOptions, + relations?: RelationPaths, + ): Promise> { + if (collectionIds.length === 0) { + return new Map(); + } + + const qb = this.listQueryBuilder.build( + ProductVariant, + { take: undefined, skip: undefined }, + { + relations: relations ?? ['taxCategory'], + channelId: ctx.channelId, + where: { deletedAt: IsNull() }, + ctx, + }, + ); + + // We explicitly join with the product to ensure we filter out soft-deleted products, + // matching the behavior of other collection-related variant queries. + qb.innerJoin('productvariant.product', 'product') + .innerJoin('productvariant.collections', 'collection', 'collection.id IN (:...collectionIds)', { + collectionIds, + }) + .andWhere('product.deletedAt IS NULL') + .addSelect('collection.id', 'collectionId') + // Using an explicit alias for the variant ID to avoid fragility of auto-generated aliases. + .addSelect('productvariant.id', 'variantId'); + + const { entities: allVariants, raw: rawResults } = await qb.getRawAndEntities(); + + // High-performance mapping using a Map for variant lookup (O(1)) + const variantsById = new Map(allVariants.map(v => [String(v.id), v])); + + const variantsByCollectionId = new Map(); + const seenInCollection = new Map>(); + + for (const id of collectionIds) { + const idStr = String(id); + variantsByCollectionId.set(idStr, []); + seenInCollection.set(idStr, new Set()); + } + + for (const raw of rawResults) { + const variantId = String(raw.variantId); + const collectionId = String(raw.collectionId); + const variant = variantsById.get(variantId); + + if (variant) { + const collectionVariants = variantsByCollectionId.get(collectionId); + const seenSet = seenInCollection.get(collectionId); + if (collectionVariants && seenSet && !seenSet.has(variantId)) { + seenSet.add(variantId); + collectionVariants.push(variant); + } + } + } + + return variantsByCollectionId; + } } diff --git a/packages/core/src/service/services/product-variant.service.ts b/packages/core/src/service/services/product-variant.service.ts index 4e91576805..248f3335de 100644 --- a/packages/core/src/service/services/product-variant.service.ts +++ b/packages/core/src/service/services/product-variant.service.ts @@ -766,11 +766,12 @@ export class ProductVariantService { } /** - * @description - * Given an array of ProductVariants from the database, this method will apply the correct price and tax - * and translate each item. - */ - private async applyPricesAndTranslateVariants( + /** + * @description + * Given an array of ProductVariants from the database, this method will apply the correct price and tax + * and translate each item. + */ + async applyPricesAndTranslateVariants( ctx: RequestContext, variants: ProductVariant[], ): Promise>> {