Fix sort added by offset not removed for MSSQL (#17343)

api/src/database/run-ast.ts

@@ -287,6 +287,9 @@ async function getDBQuery(
 	}
 
 	if (sortRecords) {
+		// Clears the order if any, eg: from MSSQL offset
+		dbQuery.clear('order');
+
 		if (needsInnerQuery) {
 			let orderByString = '';
 			const orderByFields: Knex.Raw[] = [];
@@ -326,9 +329,6 @@ async function getDBQuery(
 				);
 			}
 		} else {
-			// Clears the order if any, eg: from MSSQL offset
-			dbQuery.clear('order');
-
 			sortRecords.map((sortRecord) => {
 				if (sortRecord.column.includes('.')) {
 					const [alias, field] = sortRecord.column.split('.');

api/src/utils/apply-query.ts

@@ -39,10 +39,6 @@ export default function applyQuery(
 	const aliasMap: AliasMap = options?.aliasMap ?? Object.create(null);
 	let hasMultiRelationalFilter = false;
 
-	if (query.sort && !options?.isInnerQuery && !options?.hasMultiRelationalSort) {
-		applySort(knex, schema, dbQuery, query.sort, collection, aliasMap);
-	}
-
 	applyLimit(knex, dbQuery, query.limit);
 
 	if (query.offset) {
@@ -50,7 +46,11 @@ export default function applyQuery(
 	}
 
 	if (query.page && query.limit && query.limit !== -1) {
-		dbQuery.offset(query.limit * (query.page - 1));
+		applyOffset(knex, dbQuery, query.limit * (query.page - 1));
+	}
+
+	if (query.sort && !options?.isInnerQuery && !options?.hasMultiRelationalSort) {
+		applySort(knex, schema, dbQuery, query.sort, collection, aliasMap);
 	}
 
 	if (query.search) {
@@ -306,6 +306,9 @@ export function applySort(
 
 	if (returnRecords) return { sortRecords, hasMultiRelationalSort };
 
+	// Clears the order if any, eg: from MSSQL offset
+	rootQuery.clear('order');
+
 	rootQuery.orderBy(sortRecords);
 }
 

tests-blackbox/routes/items/no-relation.test.ts

@@ -811,6 +811,343 @@ describe.each(common.PRIMARY_KEY_TYPES)('/items', (pkType) => {
 					expect(gqlResponse.body.data[localCollectionArtists].length).toEqual(count - offset);
 				});
 			});
+
+			describe('retrieves offset with limit and sort correctly', () => {
+				it.each(vendors)('%s', async (vendor) => {
+					// Setup
+					const count = 9;
+					const offset = 4;
+					const limit = 3;
+					const sort = 'name';
+					const artistName = 'offset-limit-sort-test';
+					const artists = [];
+					const expectedResultAsc = Array.from(Array(count).keys()).slice(offset, offset + limit);
+					const expectedResultDesc = Array.from(Array(count).keys())
+						.sort((v) => -v)
+						.slice(offset, offset + limit);
+
+					for (let i = 0; i < count; i++) {
+						const artist = createArtist(pkType);
+						artist.name = `${i}-${artistName}`;
+						artists.push(artist);
+					}
+					await CreateItem(vendor, { collection: localCollectionArtists, item: artists });
+
+					// Action
+					const responseAsc = await request(getUrl(vendor))
+						.get(`/items/${localCollectionArtists}`)
+						.query({
+							filter: JSON.stringify({
+								name: { _contains: artistName },
+							}),
+							offset,
+							limit,
+							sort,
+						})
+						.set('Authorization', `Bearer ${common.USER.ADMIN.TOKEN}`);
+
+					const gqlResponseAsc = await requestGraphQL(getUrl(vendor), false, common.USER.ADMIN.TOKEN, {
+						query: {
+							[localCollectionArtists]: {
+								__args: {
+									filter: {
+										name: { _contains: artistName },
+									},
+									offset,
+									limit,
+									sort,
+								},
+								id: true,
+								name: true,
+							},
+						},
+					});
+
+					const responseDesc = await request(getUrl(vendor))
+						.get(`/items/${localCollectionArtists}`)
+						.query({
+							filter: JSON.stringify({
+								name: { _contains: artistName },
+							}),
+							offset,
+							limit,
+							sort: `-${sort}`,
+						})
+						.set('Authorization', `Bearer ${common.USER.ADMIN.TOKEN}`);
+
+					const gqlResponseDesc = await requestGraphQL(getUrl(vendor), false, common.USER.ADMIN.TOKEN, {
+						query: {
+							[localCollectionArtists]: {
+								__args: {
+									filter: {
+										name: { _contains: artistName },
+									},
+									offset,
+									limit,
+									sort: `-${sort}`,
+								},
+								id: true,
+								name: true,
+							},
+						},
+					});
+
+					// Assert
+					expect(responseAsc.statusCode).toBe(200);
+					expect(responseAsc.body.data.length).toBe(limit);
+					expect(responseAsc.body.data.map((v: any) => parseInt(v.name.split('-')[0]))).toEqual(expectedResultAsc);
+
+					expect(gqlResponseAsc.statusCode).toBe(200);
+					expect(gqlResponseAsc.body.data[localCollectionArtists].length).toEqual(limit);
+					expect(
+						gqlResponseAsc.body.data[localCollectionArtists].map((v: any) => parseInt(v.name.split('-')[0]))
+					).toEqual(expectedResultAsc);
+
+					expect(responseDesc.statusCode).toBe(200);
+					expect(responseDesc.body.data.length).toBe(limit);
+					expect(responseDesc.body.data.map((v: any) => parseInt(v.name.split('-')[0]))).toEqual(expectedResultDesc);
+
+					expect(gqlResponseDesc.statusCode).toBe(200);
+					expect(gqlResponseDesc.body.data[localCollectionArtists].length).toEqual(limit);
+					expect(
+						gqlResponseDesc.body.data[localCollectionArtists].map((v: any) => parseInt(v.name.split('-')[0]))
+					).toEqual(expectedResultDesc);
+				});
+			});
+
+			describe('retrieves offset in aggregation with limit correctly', () => {
+				it.each(vendors)('%s', async (vendor) => {
+					// Setup
+					const count = 10;
+					const offset = 3;
+					const limit = 3;
+					const groupBy = ['id', 'name'];
+					const artistName = 'offset-aggregation-limit-test';
+					const artists = [];
+
+					for (let i = 0; i < count; i++) {
+						const artist = createArtist(pkType);
+						artist.name = `${i}-${artistName}`;
+						artists.push(artist);
+					}
+					await CreateItem(vendor, { collection: localCollectionArtists, item: artists });
+
+					// Action
+					const response = await request(getUrl(vendor))
+						.get(`/items/${localCollectionArtists}`)
+						.query({
+							aggregate: {
+								count: 'id',
+							},
+							filter: JSON.stringify({
+								name: { _contains: artistName },
+							}),
+							offset,
+							limit,
+							groupBy,
+						})
+						.set('Authorization', `Bearer ${common.USER.ADMIN.TOKEN}`);
+
+					const queryKey = `${localCollectionArtists}_aggregated`;
+
+					const gqlResponse = await requestGraphQL(getUrl(vendor), false, common.USER.ADMIN.TOKEN, {
+						query: {
+							[queryKey]: {
+								__args: {
+									filter: {
+										name: { _contains: artistName },
+									},
+									offset,
+									limit,
+									groupBy,
+								},
+								count: {
+									id: true,
+								},
+								group: true,
+							},
+						},
+					});
+
+					const response2 = await request(getUrl(vendor))
+						.get(`/items/${localCollectionArtists}`)
+						.query({
+							aggregate: {
+								count: 'id',
+							},
+							filter: JSON.stringify({
+								name: { _contains: artistName },
+							}),
+							offset: offset * 2,
+							limit,
+							groupBy,
+						})
+						.set('Authorization', `Bearer ${common.USER.ADMIN.TOKEN}`);
+
+					const gqlResponse2 = await requestGraphQL(getUrl(vendor), false, common.USER.ADMIN.TOKEN, {
+						query: {
+							[queryKey]: {
+								__args: {
+									filter: {
+										name: { _contains: artistName },
+									},
+									offset: offset * 2,
+									limit,
+									groupBy,
+								},
+								count: {
+									id: true,
+								},
+								group: true,
+							},
+						},
+					});
+
+					// Assert
+					expect(response.statusCode).toBe(200);
+					expect(response.body.data.length).toBe(limit);
+
+					expect(gqlResponse.statusCode).toBe(200);
+					expect(gqlResponse.body.data[queryKey].length).toEqual(limit);
+
+					expect(response2.statusCode).toBe(200);
+					expect(response2.body.data.length).toBe(limit);
+
+					expect(gqlResponse2.statusCode).toBe(200);
+					expect(gqlResponse2.body.data[queryKey].length).toEqual(limit);
+
+					for (const item of response.body.data) {
+						expect(response2.body.data).not.toContain(item);
+					}
+
+					const gqlResults = gqlResponse.body.data[queryKey].map((v: any) => v.group.id);
+					const gqlResults2 = gqlResponse2.body.data[queryKey].map((v: any) => v.group.id);
+
+					for (const item of gqlResults) {
+						expect(gqlResults2).not.toContain(item);
+					}
+				});
+			});
+
+			describe('retrieves offset in aggregation with limit and sort correctly', () => {
+				it.each(vendors)('%s', async (vendor) => {
+					// Setup
+					const count = 10;
+					const offset = 3;
+					const limit = 6;
+					const sort = 'name';
+					const groupBy = ['id', 'name'];
+					const artistName = 'offset-aggregation-limit-sort-test';
+					const artists = [];
+					const expectedResultAsc = Array.from(Array(count).keys()).slice(offset, offset + limit);
+					const expectedResultDesc = Array.from(Array(count).keys())
+						.sort((v) => -v)
+						.slice(offset, offset + limit);
+
+					for (let i = 0; i < count; i++) {
+						const artist = createArtist(pkType);
+						artist.name = `${i}-${artistName}`;
+						artists.push(artist);
+					}
+					await CreateItem(vendor, { collection: localCollectionArtists, item: artists });
+
+					// Action
+					const responseAsc = await request(getUrl(vendor))
+						.get(`/items/${localCollectionArtists}`)
+						.query({
+							aggregate: {
+								count: 'id',
+							},
+							filter: JSON.stringify({
+								name: { _contains: artistName },
+							}),
+							offset,
+							limit,
+							sort,
+							groupBy,
+						})
+						.set('Authorization', `Bearer ${common.USER.ADMIN.TOKEN}`);
+
+					const queryKey = `${localCollectionArtists}_aggregated`;
+
+					const gqlResponseAsc = await requestGraphQL(getUrl(vendor), false, common.USER.ADMIN.TOKEN, {
+						query: {
+							[queryKey]: {
+								__args: {
+									filter: {
+										name: { _contains: artistName },
+									},
+									offset,
+									limit,
+									sort,
+									groupBy,
+								},
+								count: {
+									id: true,
+								},
+								group: true,
+							},
+						},
+					});
+
+					const responseDesc = await request(getUrl(vendor))
+						.get(`/items/${localCollectionArtists}`)
+						.query({
+							aggregate: {
+								count: 'id',
+							},
+							filter: JSON.stringify({
+								name: { _contains: artistName },
+							}),
+							offset,
+							limit,
+							sort: `-${sort}`,
+							groupBy,
+						})
+						.set('Authorization', `Bearer ${common.USER.ADMIN.TOKEN}`);
+
+					const gqlResponseDesc = await requestGraphQL(getUrl(vendor), false, common.USER.ADMIN.TOKEN, {
+						query: {
+							[queryKey]: {
+								__args: {
+									filter: {
+										name: { _contains: artistName },
+									},
+									offset,
+									limit,
+									sort: `-${sort}`,
+									groupBy,
+								},
+								count: {
+									id: true,
+								},
+								group: true,
+							},
+						},
+					});
+
+					// Assert
+					expect(responseAsc.statusCode).toBe(200);
+					expect(responseAsc.body.data.length).toBe(limit);
+					expect(responseAsc.body.data.map((v: any) => parseInt(v.name.split('-')[0]))).toEqual(expectedResultAsc);
+
+					expect(gqlResponseAsc.statusCode).toBe(200);
+					expect(gqlResponseAsc.body.data[queryKey].length).toEqual(limit);
+					expect(gqlResponseAsc.body.data[queryKey].map((v: any) => parseInt(v.group.name.split('-')[0]))).toEqual(
+						expectedResultAsc
+					);
+
+					expect(responseDesc.statusCode).toBe(200);
+					expect(responseDesc.body.data.length).toBe(limit);
+					expect(responseDesc.body.data.map((v: any) => parseInt(v.name.split('-')[0]))).toEqual(expectedResultDesc);
+
+					expect(gqlResponseDesc.statusCode).toBe(200);
+					expect(gqlResponseDesc.body.data[queryKey].length).toEqual(limit);
+					expect(gqlResponseDesc.body.data[queryKey].map((v: any) => parseInt(v.group.name.split('-')[0]))).toEqual(
+						expectedResultDesc
+					);
+				});
+			});
 		});
 	});
 });

tests-blackbox/routes/items/o2m.test.ts

@@ -1702,6 +1702,148 @@ describe.each(common.PRIMARY_KEY_TYPES)('/items', (pkType) => {
 					expect(gqlResponse.body.data[localCollectionCountries][0].states.length).toEqual(count - offset);
 				});
 			});
+
+			describe('retrieves offset with limit and sort correctly', () => {
+				it.each(vendors)('%s', async (vendor) => {
+					// Setup
+					const count = 8;
+					const offset = 3;
+					const limit = 4;
+					const sort = 'name';
+					const country = createCountry(pkType);
+					const states = [];
+					const expectedResultAsc = Array.from(Array(count).keys()).slice(offset, offset + limit);
+					const expectedResultDesc = Array.from(Array(count).keys())
+						.sort((v) => -v)
+						.slice(offset, offset + limit);
+
+					for (let i = 0; i < count; i++) {
+						const state = createState(pkType);
+						state.name = `${i}-${state.name}`;
+						states.push(state);
+					}
+
+					await CreateItem(vendor, {
+						collection: localCollectionCountries,
+						item: {
+							...country,
+							states: {
+								create: states,
+								update: [],
+								delete: [],
+							},
+						},
+					});
+
+					// Action
+					const responseAsc = await request(getUrl(vendor))
+						.get(`/items/${localCollectionCountries}`)
+						.query({
+							fields: '*.*',
+							filter: JSON.stringify({
+								name: { _eq: country.name },
+							}),
+							deep: JSON.stringify({
+								states: {
+									_offset: offset,
+									_limit: limit,
+									_sort: sort,
+								},
+							}),
+						})
+						.set('Authorization', `Bearer ${common.USER.ADMIN.TOKEN}`);
+
+					const gqlResponseAsc = await requestGraphQL(getUrl(vendor), false, common.USER.ADMIN.TOKEN, {
+						query: {
+							[localCollectionCountries]: {
+								__args: {
+									filter: {
+										name: { _eq: country.name },
+									},
+								},
+								states: {
+									__args: {
+										offset,
+										limit,
+										sort,
+									},
+									id: true,
+									name: true,
+								},
+							},
+						},
+					});
+
+					const responseDesc = await request(getUrl(vendor))
+						.get(`/items/${localCollectionCountries}`)
+						.query({
+							fields: '*.*',
+							filter: JSON.stringify({
+								name: { _eq: country.name },
+							}),
+							deep: JSON.stringify({
+								states: {
+									_offset: offset,
+									_limit: limit,
+									_sort: sort,
+								},
+							}),
+						})
+						.set('Authorization', `Bearer ${common.USER.ADMIN.TOKEN}`);
+
+					const gqlResponseDesc = await requestGraphQL(getUrl(vendor), false, common.USER.ADMIN.TOKEN, {
+						query: {
+							[localCollectionCountries]: {
+								__args: {
+									filter: {
+										name: { _eq: country.name },
+									},
+								},
+								states: {
+									__args: {
+										offset,
+										limit,
+										sort: `-${sort}`,
+									},
+									id: true,
+									name: true,
+								},
+							},
+						},
+					});
+
+					// Assert
+					expect(responseAsc.statusCode).toBe(200);
+					expect(responseAsc.body.data.length).toBe(1);
+					expect(responseAsc.body.data[0].states.length).toBe(limit);
+					expect(responseAsc.body.data[0].states.map((v: any) => parseInt(v.name.split('-')[0]))).toEqual(
+						expectedResultAsc
+					);
+
+					expect(gqlResponseAsc.statusCode).toBe(200);
+					expect(gqlResponseAsc.body.data[localCollectionCountries].length).toEqual(1);
+					expect(gqlResponseAsc.body.data[localCollectionCountries][0].states.length).toEqual(limit);
+					expect(
+						gqlResponseAsc.body.data[localCollectionCountries][0].states.map((v: any) => parseInt(v.name.split('-')[0]))
+					).toEqual(expectedResultAsc);
+
+					expect(responseDesc.statusCode).toBe(200);
+					expect(responseDesc.body.data.length).toBe(1);
+					expect(responseDesc.body.data[0].states.length).toBe(limit);
+					expect(responseDesc.body.data[0].states.map((v: any) => parseInt(v.name.split('-')[0]))).toEqual(
+						expectedResultAsc
+					);
+
+					expect(gqlResponseDesc.statusCode).toBe(200);
+					expect(gqlResponseDesc.body.data[localCollectionCountries].length).toEqual(1);
+					expect(gqlResponseDesc.body.data[localCollectionCountries][0].states.length).toEqual(limit);
+					expect(
+						gqlResponseDesc.body.data[localCollectionCountries][0].states.map((v: any) =>
+							parseInt(v.name.split('-')[0])
+						)
+					).toEqual(expectedResultDesc);
+				});
+			});
 		});
 	});
 });