Обсуждение: Do CustomScan as not projection capable node

Can we include the CustomScan node in the list of nodes that do not 
support projection?
Reason is that custom node can contain quite arbitrary logic that does 
not guarantee projection support.
Secondly. If planner does not need a separate Result node, it just 
assign tlist to subplan (i.e. changes targetlist of custom node) and 
does not change the custom_scan_tlist.
Perhaps I do not fully understand the logic of using the 
custom_scan_tlist field. But if into the PlanCustomPath() routine our 
custom node does not build own custom_scan_tlist (may be it will use 
tlist as base for custom_scan_tlist) we will get errors in the 
set_customscan_references() call.

-- 
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company

Andrey Lepikhov <a.lepikhov@postgrespro.ru> writes:
> Can we include the CustomScan node in the list of nodes that do not 
> support projection?

That seems like a pretty bad idea.  Maybe it's a good thing for whatever
unspecified extension you have in mind right now, but it's likely to be
a net negative for many more.  As an example, if some custom extension has
a better way to calculate some output expression than the core code does,
a restriction like this would prevent that from being implemented.

> Reason is that custom node can contain quite arbitrary logic that does 
> not guarantee projection support.

I don't buy this for a minute.  Where do you think projection is
going to happen?  There isn't any existing node type that *couldn't*
support projection if we insisted that that be done across-the-board.
I think it's mostly just a legacy thing that some don't.

            regards, tom lane

On Fri, Apr 19, 2019 at 12:45 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I don't buy this for a minute.  Where do you think projection is
> going to happen?  There isn't any existing node type that *couldn't*
> support projection if we insisted that that be done across-the-board.
> I think it's mostly just a legacy thing that some don't.

I think there may actually be some good reasons for that.  If
something like an Append or Material node projects, it seems to me
that this means that we built the wrong tlist for its input(s).

That justification doesn't apply to custom scans, though.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 22/04/2019 18:40, Robert Haas wrote:
> On Fri, Apr 19, 2019 at 12:45 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I don't buy this for a minute.  Where do you think projection is
>> going to happen?  There isn't any existing node type that *couldn't*
>> support projection if we insisted that that be done across-the-board.
>> I think it's mostly just a legacy thing that some don't.
> 
> I think there may actually be some good reasons for that.  If
> something like an Append or Material node projects, it seems to me
> that this means that we built the wrong tlist for its input(s).
> 
> That justification doesn't apply to custom scans, though.
The main reason for my question was incomplete information about the 
parameter custom_scan_tlist / fdw_scan_tlist.
In the process of testing my custom node, I encountered an error in 
setrefs.c caused by optimization of the projection operation. In order 
to reliably understand how to properly use custom_scan_tlist, I had to 
study in detail the mechanics of the FDW plan generator and now the 
problem is solved.
We have only three references to this parameter in the hackers mailing 
list, a brief reference on postgresql.org and limited comments into two 
patches: 1a8a4e5 and e7cb7ee.
It is possible that custom_scan_tlist is designed too nontrivially, and 
it is possible that it needs some comments describing in more detail how 
to use it.

-- 
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company

Andrey Lepikhov <a.lepikhov@postgrespro.ru> writes:
> It is possible that custom_scan_tlist is designed too nontrivially, and 
> it is possible that it needs some comments describing in more detail how 
> to use it.

I totally buy the argument that the custom scan stuff is
underdocumented :-(.

FWIW, if we did have a use-case showing that somebody would like to make
custom scans that can't project, the way to do it would be to add a flag
bit showing whether a particular CustomPath/CustomScan could project or
not.  Not to assume that they all can't.  This wouldn't be that much code
really, but I'd still like to see a plausible use-case before adding it,
because it'd be a small API break for existing CustomPath providers.

            regards, tom lane