Thread: 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
Attachment
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