From 217adc8d177dd109228bb5075fe2f6b0041eb666 Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Fri, 20 Mar 2026 15:02:35 +0100 Subject: [PATCH] fix: add missing LEFT JOIN for relation fields in groupBy orderByForRecords (#18798) ## Summary Fixes "missing FROM-clause entry for table" SQL error when using `orderByForRecords` with a relation field (e.g. `{ company: { name: "AscNullsFirst" } }`) in a `groupBy` query. ### Root cause This is a regression from #18005 (Feb 17). That PR correctly removed a duplicate `applyOrderToBuilder()` call on the groupBy subquery (which was conflicting with the `ROW_NUMBER() OVER (... ORDER BY ...)` window function), but in doing so it also removed the LEFT JOINs that `applyOrderToBuilder` was adding for relation fields. After that change, ordering relied solely on `getOrderByRawSQL()` inside `applyPartitionByToBuilder()`, which builds raw SQL for the window function but never added the required JOINs. Scalar field ordering (e.g. `name`, `position`) kept working since those don't need JOINs, but relation field ordering (e.g. `company.name`) broke. ### Fix - `getOrderByRawSQL` now returns `relationJoins` alongside the SQL string so callers get the join info they need - `applyPartitionByToBuilder` in `GroupByWithRecordsService` adds the required LEFT JOINs before building the `ROW_NUMBER()` window function, mirroring the pattern used by `applyOrderToBuilder` in the `findMany` path ## Test plan - [x] Manually tested locally with a GraphQL query matching the customer's failing pattern (`orderByForRecords: [{ company: { name: "AscNullsFirst" } }]`) - [x] Added integration tests for ascending and descending relation field ordering in `group-by-with-records-resolver.integration-spec.ts` - [x] Typecheck passes - [x] Lint passes - [x] CI green --- .../graphql-query.parser.ts | 7 +- .../services/group-by-with-records.service.ts | 24 +++- ...-with-records-resolver.integration-spec.ts | 133 ++++++++++++++++++ 3 files changed, 157 insertions(+), 7 deletions(-) diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query.parser.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query.parser.ts index e01e9bd87e..61a1f05113 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query.parser.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query.parser.ts @@ -13,6 +13,7 @@ import { GraphqlQueryOrderGroupByParser } from 'src/engine/api/graphql/graphql-q import { GraphqlQueryOrderFieldParser, type OrderByClause, + type RelationJoinInfo, } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order.parser'; import { GraphqlQuerySelectedFieldsParser, @@ -172,7 +173,7 @@ export class GraphqlQueryParser { orderBy: ObjectRecordOrderBy | OrderByWithGroupBy, objectNameSingular: string, isForwardPagination = true, - ): string { + ): { orderByRawSQL: string; relationJoins: RelationJoinInfo[] } { const parseResult = this.orderFieldParser.parse( orderBy as ObjectRecordOrderBy, objectNameSingular, @@ -208,11 +209,11 @@ export class GraphqlQueryParser { const orderByRawSQLString = orderByRawSQLClauseArray.join(', '); - const orderByCompleteSQLClause = isNonEmptyString(orderByRawSQLString) + const orderByRawSQL = isNonEmptyString(orderByRawSQLString) ? `ORDER BY ${orderByRawSQLString}` : ''; - return orderByCompleteSQLClause; + return { orderByRawSQL, relationJoins: parseResult.relationJoins }; } public applyGroupByOrderToBuilder( diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts index 5565ceff69..6ab9e963c4 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts @@ -264,12 +264,28 @@ export class GroupByWithRecordsService { flatFieldMetadataMaps, ); - const orderByRawSQL = graphqlQueryParser.getOrderByRawSQL( - orderByForRecords, - flatObjectMetadata.nameSingular, - ); + const { orderByRawSQL, relationJoins } = + graphqlQueryParser.getOrderByRawSQL( + orderByForRecords, + flatObjectMetadata.nameSingular, + ); if (isNonEmptyString(orderByRawSQL)) { + const existingJoinAliases = new Set( + queryBuilder.expressionMap.joinAttributes.map( + (joinAttribute) => joinAttribute.alias.name, + ), + ); + + for (const joinInfo of relationJoins) { + if (!existingJoinAliases.has(joinInfo.joinAlias)) { + queryBuilder.leftJoin( + `${flatObjectMetadata.nameSingular}.${joinInfo.joinAlias}`, + joinInfo.joinAlias, + ); + } + } + return queryBuilder.addSelect( `ROW_NUMBER() OVER (PARTITION BY ${groupByExpressions} ${orderByRawSQL})`, 'record_row_number', diff --git a/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-resolver.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-resolver.integration-spec.ts index 576677d372..fc698f08de 100644 --- a/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-resolver.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-resolver.integration-spec.ts @@ -565,5 +565,138 @@ describe('basic group-by with records', () => { expect(thursdayNewGroup.edges[0].node.name).toBe('Opportunity 3'); expect(thursdayNewGroup.edges[1].node.name).toBe('Opportunity 2'); }); + + it('sorts by relation field (company name) in ascending order', async () => { + const response = await makeGraphqlAPIRequest({ + query: gql` + query OpportunitiesGroupBy( + $groupBy: [OpportunityGroupByInput!]! + $filter: OpportunityFilterInput + $orderByForRecords: [OpportunityOrderByInput!] + $limit: Int + ) { + opportunitiesGroupBy( + groupBy: $groupBy + filter: $filter + orderByForRecords: $orderByForRecords + limit: $limit + ) { + groupByDimensionValues + __typename + edges { + node { + stage + name + company { + id + name + } + } + } + } + } + `, + variables: { + groupBy: [ + { + stage: true, + }, + ], + orderByForRecords: [ + { + company: { + name: 'AscNullsFirst', + }, + }, + ], + filter: FILTER_2020, + limit: 20, + }, + }); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data).toBeDefined(); + + const groups = response.body.data.opportunitiesGroupBy; + + const newGroup = groups.find((group: any) => + group.groupByDimensionValues.includes('NEW'), + ); + + expect(newGroup).toBeDefined(); + expect(newGroup.edges).toHaveLength(3); + + // Opp 1 and 2 belong to Company 1, Opp 3 to Company 2 + // Ascending by company name: Company 1 < Company 2 + expect(newGroup.edges[0].node.company.name).toBe('Company 1'); + expect(newGroup.edges[1].node.company.name).toBe('Company 1'); + expect(newGroup.edges[2].node.company.name).toBe('Company 2'); + }); + + it('sorts by relation field (company name) in descending order', async () => { + const response = await makeGraphqlAPIRequest({ + query: gql` + query OpportunitiesGroupBy( + $groupBy: [OpportunityGroupByInput!]! + $filter: OpportunityFilterInput + $orderByForRecords: [OpportunityOrderByInput!] + $limit: Int + ) { + opportunitiesGroupBy( + groupBy: $groupBy + filter: $filter + orderByForRecords: $orderByForRecords + limit: $limit + ) { + groupByDimensionValues + __typename + edges { + node { + stage + name + company { + id + name + } + } + } + } + } + `, + variables: { + groupBy: [ + { + stage: true, + }, + ], + orderByForRecords: [ + { + company: { + name: 'DescNullsLast', + }, + }, + ], + filter: FILTER_2020, + limit: 20, + }, + }); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data).toBeDefined(); + + const groups = response.body.data.opportunitiesGroupBy; + + const newGroup = groups.find((group: any) => + group.groupByDimensionValues.includes('NEW'), + ); + + expect(newGroup).toBeDefined(); + expect(newGroup.edges).toHaveLength(3); + + // Descending by company name: Company 2 > Company 1 + expect(newGroup.edges[0].node.company.name).toBe('Company 2'); + expect(newGroup.edges[1].node.company.name).toBe('Company 1'); + expect(newGroup.edges[2].node.company.name).toBe('Company 1'); + }); }); });