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: <00d461cf-c919-6a3e-2dda-c1b5d2207f3d@swarm64.com>
References: <0253344d-9bdd-11c4-7f0d-d88c02cd7991@swarm64.com>
 <347100.1610995608@sss.pgh.pa.us>
 <3ad851cf-ce22-ace0-726d-56209e214981@swarm64.com>
 <416507.1611009730@sss.pgh.pa.us>
 <00d461cf-c919-6a3e-2dda-c1b5d2207f3d@swarm64.com>
Comments: In-reply-to David Geier <david@swarm64.com>
	message dated "Tue, 19 Jan 2021 08:41:20 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <478755.1611069597.1@sss.pgh.pa.us>
Date: Tue, 19 Jan 2021 10:19:57 -0500
Message-ID: <478756.1611069597@sss.pgh.pa.us>
Precedence: bulk

David Geier <david@swarm64.com> writes:
> On 18.01.21 23:42, Tom Lane wrote:
>> OK, cool.  I was afraid you'd argue that you really needed your CustomScan
>> node to be transparent in such cases.  We could imagine inventing an
>> additional custom-scan-provider callback to embed the necessary knowledge,
>> but I'd rather not add the complexity until someone has a use-case.

> I was thinking about that. Generally, having such possibility would be 
> very useful. Unfortunately, I believe that in my specific case even 
> having such functionality wouldn't allow for the query to work with 
> CURRENT OF, because my CSP behaves similarly to a materialize node.
> My understanding is only such plans are supported which consist of nodes 
> that guarantee that the tuple returned by the plan is the unmodified 
> tuple a scan leaf node is currently positioned on?

Doesn't have to be *unmodified* --- a projection is fine, for example.
But we have to be sure that the current output tuple of the plan tree
is based on the current output tuple of the bottom-level table scan
node.  As an example of the hazards here, it's currently safe for
search_plan_tree to descend through a Limit node, but it did not use to
be, because the old implementation of Limit was such that it could return
a different tuple from the one the underlying scan node thinks it is
positioned on.

As another example, descending through Append is OK because only one
of the child scans will be positioned-on-a-tuple at all; the rest
will be at EOF or not yet started, so they can't produce a match
to whatever tuple ID the WHERE CURRENT OF is asking about.

Now that I look at this, I strongly wonder whether whoever added
MergeAppend support here understood what they were doing.  That
looks broken, because child nodes will typically be positioned on
tuples, whether or not the current top-level output came from them.
So I fear we could get a false-positive confirmation that some
tuple matches WHERE CURRENT OF.

Anyway, it seems clearly possible that some nonleaf CustomScans
would operate in a manner that would allow descending through them
while others wouldn't.  But I don't really want to write the docs
explaining what a callback for this should do ;-)

			regards, tom lane