Skip to content

Commit 0089a07

Browse files
benjiebenjie
authored
fix(pagination): fix bug in cursor pagination for PL/pgSQL SETOF… (#559)
1 parent 0a36f7b commit 0089a07

18 files changed

+378
-2
lines changed

‎packages/graphile-build-pg/src/QueryBuilder.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -770,9 +770,14 @@ with ${sql.identifier(flipAlias)} as (
770770
)
771771
select *
772772
from ${sql.identifier(flipAlias)}
773-
order by (row_number() over (partition by 1)) desc`;
773+
order by (row_number() over (partition by 1)) desc`; /* We don't need to factor useAsterisk into this row_number() usage */
774774
}
775775
if (useAsterisk) {
776+
/*
777+
* NOTE[useAsterisk/row_number]: since LIMIT/OFFSET is inside this
778+
* subquery, row_number() outside of this subquery WON'T include the
779+
* offset. We must add it back wherever row_number() is used.
780+
*/
776781
fragment = sql.fragment`select ${fields} from (${fragment}) ${this.getTableAlias()}`;
777782
}
778783
if (asJsonAggregate) {

‎packages/graphile-build-pg/src/queryFromResolveDataFactory.js‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,19 @@ exists(
297297
return sql.fragment`json_build_array(${sql.join(
298298
getPgCursorPrefix(),
299299
", "
300-
)}, (row_number() over (partition by 1)))`;
300+
)}, ${
301+
/*
302+
* NOTE[useAsterisk/row_number]: If we have useAsterisk then the
303+
* query with limit offset is in a subquery, so our row_number()
304+
* call doesn't know about it. Here we add the offset back in
305+
* again. See matching NOTE in QueryBuilder.js.
306+
*/
307+
options.useAsterisk
308+
? sql.fragment`${sql.literal(
309+
queryBuilder.getFinalOffset() || 0
310+
)} + `
311+
: sql.fragment``
312+
}(row_number() over (partition by 1)))`;
301313
}
302314
});
303315
}

‎packages/postgraphile-core/__tests__/fixtures/queries/procedure-query.graphql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ query {
3838
tableSetQueryWithOffset10: tableSetQuery(before: "WyJuYXR1cmFsIiwxXQ==", last: 2) { edges { cursor node { name } } pageInfo { startCursor endCursor hasNextPage hasPreviousPage } }
3939
# Using after and offset together should not throw
4040
tableSetQueryWithOffset11: tableSetQuery(after: "WyJuYXR1cmFsIiwxXQ==", offset: 1, first: 2) { edges { cursor node { name } } pageInfo { startCursor endCursor hasNextPage hasPreviousPage } }
41+
tableSetQueryPlpgsql(first:2) { edges { cursor node { name } } pageInfo { startCursor endCursor hasNextPage hasPreviousPage } }
42+
tableSetQueryPlpgsqlOffset2: tableSetQueryPlpgsql(first:2, after: "WyJuYXR1cmFsIiwyXQ==") { edges { cursor node { name } } pageInfo { startCursor endCursor hasNextPage hasPreviousPage } }
4143
intSetQuery(x: 5, z: 6) { edges { cursor node } totalCount }
4244
noArgsQuery
4345
staticBigInteger { edges { node } totalCount }

‎packages/postgraphile-core/__tests__/integration/__snapshots__/queries.test.js.snap‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5299,6 +5299,50 @@ Object {
52995299
"startCursor": "WyJuYXR1cmFsIiwxXQ==",
53005300
},
53015301
},
5302+
"tableSetQueryPlpgsql": Object {
5303+
"edges": Array [
5304+
Object {
5305+
"cursor": "WyJuYXR1cmFsIiwxXQ==",
5306+
"node": Object {
5307+
"name": "John Smith",
5308+
},
5309+
},
5310+
Object {
5311+
"cursor": "WyJuYXR1cmFsIiwyXQ==",
5312+
"node": Object {
5313+
"name": "Sara Smith",
5314+
},
5315+
},
5316+
],
5317+
"pageInfo": Object {
5318+
"endCursor": "WyJuYXR1cmFsIiwyXQ==",
5319+
"hasNextPage": true,
5320+
"hasPreviousPage": false,
5321+
"startCursor": "WyJuYXR1cmFsIiwxXQ==",
5322+
},
5323+
},
5324+
"tableSetQueryPlpgsqlOffset2": Object {
5325+
"edges": Array [
5326+
Object {
5327+
"cursor": "WyJuYXR1cmFsIiwzXQ==",
5328+
"node": Object {
5329+
"name": "Budd Deey",
5330+
},
5331+
},
5332+
Object {
5333+
"cursor": "WyJuYXR1cmFsIiw0XQ==",
5334+
"node": Object {
5335+
"name": "Kathryn Ramirez",
5336+
},
5337+
},
5338+
],
5339+
"pageInfo": Object {
5340+
"endCursor": "WyJuYXR1cmFsIiw0XQ==",
5341+
"hasNextPage": true,
5342+
"hasPreviousPage": true,
5343+
"startCursor": "WyJuYXR1cmFsIiwzXQ==",
5344+
},
5345+
},
53025346
"tableSetQueryWithOffset": Object {
53035347
"edges": Array [
53045348
Object {

‎packages/postgraphile-core/__tests__/integration/schema/__snapshots__/defaultOptions.test.js.snap‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7645,6 +7645,27 @@ type Query implements Node {
76457645
orderBy: [PeopleOrderBy!]
76467646
): PeopleConnection!
76477647

7648+
"""Reads and enables pagination through a set of \`Person\`."""
7649+
tableSetQueryPlpgsql(
7650+
"""Read all values in the set after (below) this cursor."""
7651+
after: Cursor
7652+
7653+
"""Read all values in the set before (above) this cursor."""
7654+
before: Cursor
7655+
7656+
"""Only read the first \`n\` values of the set."""
7657+
first: Int
7658+
7659+
"""Only read the last \`n\` values of the set."""
7660+
last: Int
7661+
7662+
"""
7663+
Skip the first \`n\` values from our \`after\` cursor, an alternative to cursor
7664+
based pagination. May not be used with \`last\`.
7665+
"""
7666+
offset: Int
7667+
): PeopleConnection!
7668+
76487669
"""Reads a single \`Type\` using its globally unique \`ID\`."""
76497670
type(
76507671
"""The globally unique \`ID\` to be used in selecting a single \`Type\`."""
@@ -17704,6 +17725,27 @@ type Query implements Node {
1770417725
orderBy: [PeopleOrderBy!]
1770517726
): PeopleConnection!
1770617727

17728+
"""Reads and enables pagination through a set of \`Person\`."""
17729+
tableSetQueryPlpgsql(
17730+
"""Read all values in the set after (below) this cursor."""
17731+
after: Cursor
17732+
17733+
"""Read all values in the set before (above) this cursor."""
17734+
before: Cursor
17735+
17736+
"""Only read the first \`n\` values of the set."""
17737+
first: Int
17738+
17739+
"""Only read the last \`n\` values of the set."""
17740+
last: Int
17741+
17742+
"""
17743+
Skip the first \`n\` values from our \`after\` cursor, an alternative to cursor
17744+
based pagination. May not be used with \`last\`.
17745+
"""
17746+
offset: Int
17747+
): PeopleConnection!
17748+
1770717749
"""Reads a single \`Type\` using its globally unique \`ID\`."""
1770817750
type(
1770917751
"""The globally unique \`ID\` to be used in selecting a single \`Type\`."""

‎packages/postgraphile-core/__tests__/integration/schema/__snapshots__/disabledTagsNoLegacyRelations.test.js.snap‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3769,6 +3769,27 @@ type Query implements Node {
37693769
"""
37703770
offset: Int
37713771
): PeopleConnection!
3772+
3773+
"""Reads and enables pagination through a set of \`Person\`."""
3774+
tableSetQueryPlpgsql(
3775+
"""Read all values in the set after (below) this cursor."""
3776+
after: Cursor
3777+
3778+
"""Read all values in the set before (above) this cursor."""
3779+
before: Cursor
3780+
3781+
"""Only read the first \`n\` values of the set."""
3782+
first: Int
3783+
3784+
"""Only read the last \`n\` values of the set."""
3785+
last: Int
3786+
3787+
"""
3788+
Skip the first \`n\` values from our \`after\` cursor, an alternative to cursor
3789+
based pagination. May not be used with \`last\`.
3790+
"""
3791+
offset: Int
3792+
): PeopleConnection!
37723793
typesQuery(a: BigInt!, b: Boolean!, c: String!, d: [Int]!, e: JSON!, f: FloatRangeInput!): Boolean
37733794
}
37743795

‎packages/postgraphile-core/__tests__/integration/schema/__snapshots__/function-clash.test.js.snap‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7645,6 +7645,27 @@ type Query implements Node {
76457645
orderBy: [PeopleOrderBy!]
76467646
): PeopleConnection!
76477647
7648+
"""Reads and enables pagination through a set of \`Person\`."""
7649+
tableSetQueryPlpgsql(
7650+
"""Read all values in the set after (below) this cursor."""
7651+
after: Cursor
7652+
7653+
"""Read all values in the set before (above) this cursor."""
7654+
before: Cursor
7655+
7656+
"""Only read the first \`n\` values of the set."""
7657+
first: Int
7658+
7659+
"""Only read the last \`n\` values of the set."""
7660+
last: Int
7661+
7662+
"""
7663+
Skip the first \`n\` values from our \`after\` cursor, an alternative to cursor
7664+
based pagination. May not be used with \`last\`.
7665+
"""
7666+
offset: Int
7667+
): PeopleConnection!
7668+
76487669
"""Reads a single \`Type\` using its globally unique \`ID\`."""
76497670
type(
76507671
"""The globally unique \`ID\` to be used in selecting a single \`Type\`."""

‎packages/postgraphile-core/__tests__/integration/schema/__snapshots__/indexes.test.js.snap‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7835,6 +7835,27 @@ type Query implements Node {
78357835
orderBy: [PeopleOrderBy!]
78367836
): PeopleConnection!
78377837

7838+
"""Reads and enables pagination through a set of \`Person\`."""
7839+
tableSetQueryPlpgsql(
7840+
"""Read all values in the set after (below) this cursor."""
7841+
after: Cursor
7842+
7843+
"""Read all values in the set before (above) this cursor."""
7844+
before: Cursor
7845+
7846+
"""Only read the first \`n\` values of the set."""
7847+
first: Int
7848+
7849+
"""Only read the last \`n\` values of the set."""
7850+
last: Int
7851+
7852+
"""
7853+
Skip the first \`n\` values from our \`after\` cursor, an alternative to cursor
7854+
based pagination. May not be used with \`last\`.
7855+
"""
7856+
offset: Int
7857+
): PeopleConnection!
7858+
78387859
"""Reads a single \`Type\` using its globally unique \`ID\`."""
78397860
type(
78407861
"""The globally unique \`ID\` to be used in selecting a single \`Type\`."""

‎packages/postgraphile-core/__tests__/integration/schema/__snapshots__/inflect-core.test.js.snap‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7650,6 +7650,27 @@ type Q implements N {
76507650
orderBy: [PeopleOrderBy!]
76517651
): PeopleConnection!
76527652

7653+
"""Reads and enables pagination through a set of \`Person\`."""
7654+
tableSetQueryPlpgsql(
7655+
"""Read all values in the set after (below) this cursor."""
7656+
after: Cursor
7657+
7658+
"""Read all values in the set before (above) this cursor."""
7659+
before: Cursor
7660+
7661+
"""Only read the first \`n\` values of the set."""
7662+
first: Int
7663+
7664+
"""Only read the last \`n\` values of the set."""
7665+
last: Int
7666+
7667+
"""
7668+
Skip the first \`n\` values from our \`after\` cursor, an alternative to cursor
7669+
based pagination. May not be used with \`last\`.
7670+
"""
7671+
offset: Int
7672+
): PeopleConnection!
7673+
76537674
"""Reads a single \`Type\` using its globally unique \`ID\`."""
76547675
type(
76557676
"""The globally unique \`ID\` to be used in selecting a single \`Type\`."""

‎packages/postgraphile-core/__tests__/integration/schema/__snapshots__/legacyFunctionsOnly.test.js.snap‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2916,6 +2916,27 @@ type Query implements Node {
29162916
"""The method to use when ordering \`Person\`."""
29172917
orderBy: [PeopleOrderBy!]
29182918
): PeopleConnection!
2919+
2920+
"""Reads and enables pagination through a set of \`Person\`."""
2921+
tableSetQueryPlpgsql(
2922+
"""Read all values in the set after (below) this cursor."""
2923+
after: Cursor
2924+
2925+
"""Read all values in the set before (above) this cursor."""
2926+
before: Cursor
2927+
2928+
"""Only read the first \`n\` values of the set."""
2929+
first: Int
2930+
2931+
"""Only read the last \`n\` values of the set."""
2932+
last: Int
2933+
2934+
"""
2935+
Skip the first \`n\` values from our \`after\` cursor, an alternative to cursor
2936+
based pagination. May not be used with \`last\`.
2937+
"""
2938+
offset: Int
2939+
): PeopleConnection!
29192940
typesQuery(a: BigInt!, b: Boolean!, c: String!, d: [Int]!, e: JSON!, f: FloatRangeInput!): Boolean
29202941
}
29212942

0 commit comments

Comments
 (0)