Hi,
On 18.01.21 19:46, Tom Lane wrote:
David Geier <da...@swarm64.com> writes:
search_plan_tree() assumes that
CustomScanState::ScanState::ss_currentRelation is never NULL. In my
understanding that only holds for CustomScanState nodes which are at the
bottom of the plan and actually read from a relation. CustomScanState
nodes which are not at the bottom don't have ss_currentRelation set. I
believe for such nodes, instead search_plan_tree() should recurse into
CustomScanState::custom_ps.
Hm. I agree that we shouldn't simply assume that ss_currentRelation
isn't null. However, we cannot make search_plan_tree() descend
through non-leaf CustomScan nodes, because we don't know what processing
is involved there. We need to find a scan that is guaranteed to return
rows that are one-to-one with the cursor output. This is why the function
doesn't descend through join or aggregation nodes, and I see no argument
by which we should assume we know more about what a customscan node will
do than we know about those.
That makes sense. Thanks for the explanation.
So I'm inclined to think a suitable fix is just
- if (RelationGetRelid(sstate->ss_currentRelation) == table_oid)
+ if (sstate->ss_currentRelation &&
+ RelationGetRelid(sstate->ss_currentRelation) == table_oid)
result = sstate;
regards, tom lane
I updated the patch to match your proposal.
Best regards,
David
Swarm64
diff --git a/src/backend/executor/execCurrent.c
b/src/backend/executor/execCurrent.c
index c7f909241b..fb3cf9537c 100644
--- a/src/backend/executor/execCurrent.c
+++ b/src/backend/executor/execCurrent.c
@@ -330,7 +330,8 @@ search_plan_tree(PlanState *node, Oid table_oid,
{
ScanState *sstate = (ScanState *) node;
- if
(RelationGetRelid(sstate->ss_currentRelation) == table_oid)
+ if (sstate->ss_currentRelation &&
+
RelationGetRelid(sstate->ss_currentRelation) == table_oid)
result = sstate;
break;
}
