[PATCH] Allow CustomScan nodes to signal projection support

Started by Sven Klemmabout 5 years ago3 messageshackers
Jump to latest
#1Sven Klemm
sven@timescale.com

Hi,

The attached patch allows CustomScan nodes to signal whether they
support projection.
Currently all CustomScan nodes are treated as supporting projection.
But it would be nice
for custom nodes to opt out of this to prevent postgres from modifying
the targetlist of
the custom node.

For features similar to set-returning functions the function call
needs to be a top level expression in a custom node that implements it
but any targetlist adjustments
might be modified by postgres because it is not aware of the special
meaning of those function calls and it might push down a different
targetlist in the node cause it assumes
the node can project.

I named the flag CUSTOMPATH_SUPPORT_PROJECTION similar to the other
custom node flags, but this would revert the current logic and nodes
would have to opt into
projection. I thought about naming it CUSTOMPATH_CANNOT_PROJECT to keep
the current default and make it an opt out. But that would make the
flag name notably different from the other flag names. Any opinions on
the flag name and whether it should be opt in or opt out?

--
Regards, Sven Klemm

Attachments:

v1-0001-costumscan_projection_flag.patchapplication/octet-stream; name=v1-0001-costumscan_projection_flag.patchDownload+5-1
#2Aleksander Alekseev
aleksander@timescale.com
In reply to: Sven Klemm (#1)
Re: [PATCH] Allow CustomScan nodes to signal projection support

Hi Sven,

The attached patch allows CustomScan nodes to signal whether they
support projection.

I noticed that you didn't change custom-scan.sgml accordingly. The
updated patch is attached. Otherwise, it seems to be fine in terms of
compiling, passing tests etc.

I named the flag CUSTOMPATH_SUPPORT_PROJECTION similar to the other
custom node flags, but this would revert the current logic

This seems to be a typical Kobayashi Maru situation, i.e any choice is
a bad one. I suggest keeping the patch as is and hoping that the
developers of existing extensions read the release notes.

--
Best regards,
Aleksander Alekseev

Attachments:

v2-0001-costumscan_projection_flag.patchapplication/octet-stream; name=v2-0001-costumscan_projection_flag.patchDownload+8-2
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Aleksander Alekseev (#2)
Re: [PATCH] Allow CustomScan nodes to signal projection support

Aleksander Alekseev <aleksander@timescale.com> writes:

I named the flag CUSTOMPATH_SUPPORT_PROJECTION similar to the other
custom node flags, but this would revert the current logic

This seems to be a typical Kobayashi Maru situation, i.e any choice is
a bad one. I suggest keeping the patch as is and hoping that the
developers of existing extensions read the release notes.

Yeah, I concur that's the least bad choice.

I got annoyed by the fact that the existing checks of CustomPath flags
had several randomly different styles for basically identical tests,
and this patch wanted to introduce yet another. I cleaned that up and
pushed this.

regards, tom lane