Skip to content

Commit 4e65df2

Browse files
committed
fix: materialize scroll cursor results when ROWNUM present
For scroll cursors containing ROWNUM expressions, materialize all results into a tuplestore on first execution. This ensures FETCH PRIOR/NEXT/RELATIVE return the same ROWNUM values computed during initial scan, rather than re-evaluating (which caused incorrect incrementing values). Added plan_contains_rownum() to detect ROWNUM in plan trees, handling SubqueryScan, Append, and MergeAppend nodes. Fixes: IvorySQL#1000 (comment)
1 parent 387dfae commit 4e65df2

File tree

2 files changed

+144
-3
lines changed

2 files changed

+144
-3
lines changed

src/backend/tcop/pquery.c

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include "executor/executor.h"
2323
#include "executor/tstoreReceiver.h"
2424
#include "miscadmin.h"
25+
#include "nodes/nodeFuncs.h"
26+
#include "nodes/plannodes.h"
2527
#include "pg_trace.h"
2628
#include "tcop/pquery.h"
2729
#include "tcop/utility.h"
@@ -60,6 +62,130 @@ static uint64 DoPortalRunFetch(Portal portal,
6062
DestReceiver *dest);
6163
static void DoPortalRewind(Portal portal);
6264
static TupleDesc CreateTupleDescFromParams(ParamListInfo params);
65+
static bool plan_contains_rownum_walker(Node *node, void *context);
66+
static bool plan_contains_rownum(Plan *plan);
67+
static void FillPortalStoreForSelect(Portal portal);
68+
69+
/*
70+
* Helper function to check if an expression contains ROWNUM
71+
*/
72+
static bool
73+
plan_contains_rownum_walker(Node *node, void *context)
74+
{
75+
if (node == NULL)
76+
return false;
77+
78+
if (IsA(node, RownumExpr))
79+
return true;
80+
81+
return expression_tree_walker(node, plan_contains_rownum_walker, context);
82+
}
83+
84+
/*
85+
* Check if a plan tree contains ROWNUM expressions in targetlists.
86+
* This is used to determine if scroll cursor results need materialization.
87+
*/
88+
static bool
89+
plan_contains_rownum(Plan *plan)
90+
{
91+
ListCell *lc;
92+
93+
if (plan == NULL)
94+
return false;
95+
96+
/* Check the plan's targetlist */
97+
foreach(lc, plan->targetlist)
98+
{
99+
TargetEntry *tle = lfirst_node(TargetEntry, lc);
100+
101+
if (plan_contains_rownum_walker((Node *) tle->expr, NULL))
102+
return true;
103+
}
104+
105+
/* Recursively check standard child plans */
106+
if (plan->lefttree && plan_contains_rownum(plan->lefttree))
107+
return true;
108+
if (plan->righttree && plan_contains_rownum(plan->righttree))
109+
return true;
110+
111+
/* Handle special plan node types with additional child plans */
112+
switch (nodeTag(plan))
113+
{
114+
case T_SubqueryScan:
115+
{
116+
SubqueryScan *splan = (SubqueryScan *) plan;
117+
if (plan_contains_rownum(splan->subplan))
118+
return true;
119+
}
120+
break;
121+
122+
case T_Append:
123+
{
124+
Append *aplan = (Append *) plan;
125+
foreach(lc, aplan->appendplans)
126+
{
127+
if (plan_contains_rownum((Plan *) lfirst(lc)))
128+
return true;
129+
}
130+
}
131+
break;
132+
133+
case T_MergeAppend:
134+
{
135+
MergeAppend *mplan = (MergeAppend *) plan;
136+
foreach(lc, mplan->mergeplans)
137+
{
138+
if (plan_contains_rownum((Plan *) lfirst(lc)))
139+
return true;
140+
}
141+
}
142+
break;
143+
144+
default:
145+
/* Most plan nodes only have lefttree/righttree children */
146+
break;
147+
}
148+
149+
return false;
150+
}
151+
152+
/*
153+
* FillPortalStoreForSelect
154+
* Run a PORTAL_ONE_SELECT query and store results in portal's holdStore.
155+
*
156+
* This is used for scroll cursors that contain ROWNUM expressions.
157+
* We need to materialize results so that FETCH PRIOR/NEXT don't re-evaluate
158+
* ROWNUM values.
159+
*/
160+
static void
161+
FillPortalStoreForSelect(Portal portal)
162+
{
163+
QueryDesc *queryDesc = portal->queryDesc;
164+
DestReceiver *treceiver;
165+
166+
Assert(queryDesc != NULL);
167+
Assert(portal->holdStore == NULL);
168+
169+
PortalCreateHoldStore(portal);
170+
treceiver = CreateDestReceiver(DestTuplestore);
171+
SetTuplestoreDestReceiverParams(treceiver,
172+
portal->holdStore,
173+
portal->holdContext,
174+
false,
175+
NULL,
176+
NULL);
177+
178+
/* Run the query to completion, storing results in tuplestore */
179+
PushActiveSnapshot(queryDesc->snapshot);
180+
queryDesc->dest = treceiver;
181+
ExecutorRun(queryDesc, ForwardScanDirection, 0);
182+
PopActiveSnapshot();
183+
184+
treceiver->rDestroy(treceiver);
185+
186+
/* Reset the tuplestore to start */
187+
tuplestore_rescan(portal->holdStore);
188+
}
63189

64190
/*
65191
* CreateQueryDesc
@@ -892,6 +1018,21 @@ PortalRunSelect(Portal portal,
8921018
/* Caller messed up if we have neither a ready query nor held data. */
8931019
Assert(queryDesc || portal->holdStore);
8941020

1021+
/*
1022+
* For scroll cursors with ROWNUM in the plan, we need to materialize
1023+
* all results on first execution. This ensures that FETCH PRIOR/NEXT
1024+
* return the same ROWNUM values that were computed during the initial
1025+
* forward scan, rather than re-evaluating ROWNUM (which would give
1026+
* incorrect incrementing values).
1027+
*/
1028+
if (queryDesc != NULL &&
1029+
portal->holdStore == NULL &&
1030+
(portal->cursorOptions & CURSOR_OPT_SCROLL) &&
1031+
plan_contains_rownum(queryDesc->plannedstmt->planTree))
1032+
{
1033+
FillPortalStoreForSelect(portal);
1034+
}
1035+
8951036
/*
8961037
* Force the queryDesc destination to the right thing. This supports
8971038
* MOVE, for example, which will pass in dest = DestNone. This is okay to

src/oracle_test/regress/expected/rownum.out

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,14 +1509,14 @@ FETCH LAST IN cursor_rn;
15091509
FETCH PRIOR IN cursor_rn;
15101510
c1 | c2 | rn
15111511
----+----+----
1512-
7 | 7 | 9
1512+
7 | 7 | 7
15131513
(1 row)
15141514

15151515
-- FETCH NEXT should return row with rn=8 again (not a new incremented value)
15161516
FETCH NEXT IN cursor_rn;
15171517
c1 | c2 | rn
15181518
----+----+----
1519-
8 | 8 | 10
1519+
8 | 8 | 8
15201520
(1 row)
15211521

15221522
-- FETCH ABSOLUTE 2 should return row with rn=5
@@ -1530,7 +1530,7 @@ FETCH ABSOLUTE 2 IN cursor_rn;
15301530
FETCH RELATIVE -1 IN cursor_rn;
15311531
c1 | c2 | rn
15321532
----+----+----
1533-
4 | 4 | 6
1533+
4 | 4 | 4
15341534
(1 row)
15351535

15361536
CLOSE cursor_rn;

0 commit comments

Comments
 (0)