Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0ab177b

Browse files
committed
Remove faulty support for MergeAppend plan with WHERE CURRENT OF.
Somebody extended search_plan_tree() to treat MergeAppend exactlylike Append, which is 100% wrong, because unlike Append we can'tassume that only one input node is actively returning tuples.Hence a cursor using a MergeAppend across a UNION ALL or inheritancetree could falsely match a WHERE CURRENT OF query at a row thatisn't actually the cursor's current output row, but coincidentallyhas the same TID (in a different table) as the current output row.Delete the faulty code; this means that such a case will now returnan error like 'cursor "foo" is not a simply updatable scan of table"bar"', instead of silently misbehaving. Users should not find thatsurprising though, as the same cursor query could have failed that wayalready depending on the chosen plan. (It would fail like that if thesort were done with an explicit Sort node instead of MergeAppend.)Expand the clearly-inadequate commentary to be more explicit aboutwhat this code is doing, in hopes of forestalling future mistakes.It's been like this for awhile, so back-patch to all supportedbranches.Discussion:https://postgr.es/m/482865.1611075182@sss.pgh.pa.us
1 parent33a4699 commit0ab177b

File tree

1 file changed

+27
-26
lines changed

1 file changed

+27
-26
lines changed

‎src/backend/executor/execCurrent.c

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,10 @@ fetch_cursor_param_value(ExprContext *econtext, int paramId)
296296
* Search through a PlanState tree for a scan node on the specified table.
297297
* Return NULL if not found or multiple candidates.
298298
*
299+
* CAUTION: this function is not charged simply with finding some candidate
300+
* scan, but with ensuring that that scan returned the plan tree's current
301+
* output row. That's why we must reject multiple-match cases.
302+
*
299303
* If a candidate is found, set *pending_rescan to true if that candidate
300304
* or any node above it has a pending rescan action, i.e. chgParam != NULL.
301305
* That indicates that we shouldn't consider the node to be positioned on a
@@ -314,7 +318,9 @@ search_plan_tree(PlanState *node, Oid table_oid,
314318
switch (nodeTag(node))
315319
{
316320
/*
317-
* Relation scan nodes can all be treated alike. Note that
321+
* Relation scan nodes can all be treated alike: check to see if
322+
* they are scanning the specified table.
323+
*
318324
* ForeignScan and CustomScan might not have a currentRelation, in
319325
* which case we just ignore them. (We dare not descend to any
320326
* child plan nodes they might have, since we do not know the
@@ -339,8 +345,26 @@ search_plan_tree(PlanState *node, Oid table_oid,
339345
}
340346

341347
/*
342-
* For Append, we must look through the members; watch out for
343-
* multiple matches (possible if it was from UNION ALL)
348+
* For Append, we can check each input node. It is safe to
349+
* descend to the inputs because only the input that resulted in
350+
* the Append's current output node could be positioned on a tuple
351+
* at all; the other inputs are either at EOF or not yet started.
352+
* Hence, if the desired table is scanned by some
353+
* currently-inactive input node, we will find that node but then
354+
* our caller will realize that it didn't emit the tuple of
355+
* interest.
356+
*
357+
* We do need to watch out for multiple matches (possible if
358+
* Append was from UNION ALL rather than an inheritance tree).
359+
*
360+
* Note: we can NOT descend through MergeAppend similarly, since
361+
* its inputs are likely all active, and we don't know which one
362+
* returned the current output tuple. (Perhaps that could be
363+
* fixed if we were to let this code know more about MergeAppend's
364+
* internal state, but it does not seem worth the trouble. Users
365+
* should not expect plans for ORDER BY queries to be considered
366+
* simply-updatable, since they won't be if the sorting is
367+
* implemented by a Sort node.)
344368
*/
345369
caseT_AppendState:
346370
{
@@ -362,29 +386,6 @@ search_plan_tree(PlanState *node, Oid table_oid,
362386
break;
363387
}
364388

365-
/*
366-
* Similarly for MergeAppend
367-
*/
368-
caseT_MergeAppendState:
369-
{
370-
MergeAppendState*mstate= (MergeAppendState*)node;
371-
inti;
372-
373-
for (i=0;i<mstate->ms_nplans;i++)
374-
{
375-
ScanState*elem=search_plan_tree(mstate->mergeplans[i],
376-
table_oid,
377-
pending_rescan);
378-
379-
if (!elem)
380-
continue;
381-
if (result)
382-
returnNULL;/* multiple matches */
383-
result=elem;
384-
}
385-
break;
386-
}
387-
388389
/*
389390
* Result and Limit can be descended through (these are safe
390391
* because they always return their input's current row)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp