Re: Do CustomScan as not projection capable node
Andrey Lepikhov 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
Re: Do CustomScan as not projection capable node
On 22/04/2019 18:40, Robert Haas wrote: On Fri, Apr 19, 2019 at 12:45 AM Tom Lane 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
Re: Do CustomScan as not projection capable node
On Fri, Apr 19, 2019 at 12:45 AM Tom Lane 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
Re: Do CustomScan as not projection capable node
Andrey Lepikhov 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
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 >From 938904d179e0a4e31cbb20fb70243d2b980d8dc2 Mon Sep 17 00:00:00 2001 From: "Andrey V. Lepikhov" Date: Fri, 19 Apr 2019 09:34:09 +0500 Subject: [PATCH] Add CustomScan node in the list of nodes that do not support projection --- src/backend/optimizer/plan/createplan.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index efe073a3ee..682d4d4429 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -6691,6 +6691,7 @@ is_projection_capable_path(Path *path) case T_ModifyTable: case T_MergeAppend: case T_RecursiveUnion: + case T_CustomScan: return false; case T_Append: -- 2.17.1
