From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Geier <david@swarm64.com>
cc: pgsql-hackers@postgresql.org
Subject: Re: search_plan_tree(): handling of non-leaf CustomScanState nodes
 causes segfault
In-reply-to: <0253344d-9bdd-11c4-7f0d-d88c02cd7991@swarm64.com>
References: <0253344d-9bdd-11c4-7f0d-d88c02cd7991@swarm64.com>
Comments: In-reply-to David Geier <david@swarm64.com>
	message dated "Mon, 18 Jan 2021 11:43:30 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <347099.1610995608.1@sss.pgh.pa.us>
Date: Mon, 18 Jan 2021 13:46:48 -0500
Message-ID: <347100.1610995608@sss.pgh.pa.us>
Precedence: bulk

David Geier <david@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.

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