Re: search_plan_tree(): handling of non-leaf CustomScanState nodes causes segfault

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Geier <david(at)swarm64(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: search_plan_tree(): handling of non-leaf CustomScanState nodes causes segfault
Date: 2021-01-18 18:46:48
Message-ID: 347100.1610995608@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Geier <david(at)swarm64(dot)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.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Surafel Temesgen 2021-01-18 18:56:13 Re: WIP: System Versioned Temporal Table
Previous Message Justin Pryzby 2021-01-18 18:34:59 Re: CLUSTER on partitioned index