[PATCH] Allow specification of custom slot for custom nodes
Hackers,
we have supported custom nodes for while. Custom slots are a bit more
"recent" feature. Since we now support custom slots, which could
handle custom tuple format, why not allow custom nodes to use them?
For instance, a custom table access method can have its own tuple
format and use a custom node to provide some custom type of scan. The
ability to set a custom slot would save us from tuple format
conversion (thank happened to me while working on OrioleDB). I think
other users of custom nodes may also benefit.
------
Regards,
Alexander Korotkov
Attachments:
0001-Allow-specification-of-custom-slot-for-custom-nod-v1.patchapplication/octet-stream; name=0001-Allow-specification-of-custom-slot-for-custom-nod-v1.patchDownload
From d9fe25cd6806315894097b83929eeedaae4f2db0 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Thu, 30 Jun 2022 22:33:18 +0300
Subject: [PATCH] Allow specification of custom slot for custom nodes
---
src/backend/executor/nodeCustom.c | 13 +++++++++++--
src/include/nodes/execnodes.h | 1 +
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/backend/executor/nodeCustom.c b/src/backend/executor/nodeCustom.c
index 8f56bd8a23a..a76ec43b9fe 100644
--- a/src/backend/executor/nodeCustom.c
+++ b/src/backend/executor/nodeCustom.c
@@ -29,6 +29,7 @@ CustomScanState *
ExecInitCustomScan(CustomScan *cscan, EState *estate, int eflags)
{
CustomScanState *css;
+ const TupleTableSlotOps *slotOps;
Relation scan_rel = NULL;
Index scanrelid = cscan->scan.scanrelid;
int tlistvarno;
@@ -63,6 +64,14 @@ ExecInitCustomScan(CustomScan *cscan, EState *estate, int eflags)
css->ss.ss_currentRelation = scan_rel;
}
+ /*
+ * Use a custom slot if specified in CustomScanState or use virtual slot
+ * otherwise.
+ */
+ slotOps = css->slotOps;
+ if (!slotOps)
+ slotOps = &TTSOpsVirtual;
+
/*
* Determine the scan tuple type. If the custom scan provider provided a
* targetlist describing the scan tuples, use that; else use base
@@ -73,14 +82,14 @@ ExecInitCustomScan(CustomScan *cscan, EState *estate, int eflags)
TupleDesc scan_tupdesc;
scan_tupdesc = ExecTypeFromTL(cscan->custom_scan_tlist);
- ExecInitScanTupleSlot(estate, &css->ss, scan_tupdesc, &TTSOpsVirtual);
+ ExecInitScanTupleSlot(estate, &css->ss, scan_tupdesc, slotOps);
/* Node's targetlist will contain Vars with varno = INDEX_VAR */
tlistvarno = INDEX_VAR;
}
else
{
ExecInitScanTupleSlot(estate, &css->ss, RelationGetDescr(scan_rel),
- &TTSOpsVirtual);
+ slotOps);
/* Node's targetlist will contain Vars with varno = scanrelid */
tlistvarno = scanrelid;
}
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 57288013795..2eebf60c8c3 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1932,6 +1932,7 @@ typedef struct CustomScanState
List *custom_ps; /* list of child PlanState nodes, if any */
Size pscan_len; /* size of parallel coordination information */
const struct CustomExecMethods *methods;
+ const struct TupleTableSlotOps *slotOps;
} CustomScanState;
/* ----------------------------------------------------------------
--
2.24.3 (Apple Git-128)
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested
I've looked at this patch and don't see any problems with it. It is minimally invasive, it doesn't affect functionality unless anyone (e.g. extension) sets its own slotOps in CustomScanState.
Furthermore, the current patch very slightly modifies patch 0b03e5951bf0 with the intention of introducing extensibility. So I think adding more extensibility regarding different tuple formats is an excellent thing to do.
I'm going to mark it as RfC if there are no objections.
Kind regards,
Pavel Borisov,
Supabase
The new status of this patch is: Ready for Committer
On Mon, Nov 21, 2022 at 4:34 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not testedI've looked at this patch and don't see any problems with it. It is minimally invasive, it doesn't affect functionality unless anyone (e.g. extension) sets its own slotOps in CustomScanState.
Furthermore, the current patch very slightly modifies patch 0b03e5951bf0 with the intention of introducing extensibility. So I think adding more extensibility regarding different tuple formats is an excellent thing to do.I'm going to mark it as RfC if there are no objections.
Thank you for your feedback. I also don't see how this patch could
affect anybody.
I'm going to push this if there are no objections.
------
Regards,
Alexander Korotkov
2022年11月22日(火) 5:50 Alexander Korotkov <aekorotkov@gmail.com>:
On Mon, Nov 21, 2022 at 4:34 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not testedI've looked at this patch and don't see any problems with it. It is minimally invasive, it doesn't affect functionality unless anyone (e.g. extension) sets its own slotOps in CustomScanState.
Furthermore, the current patch very slightly modifies patch 0b03e5951bf0 with the intention of introducing extensibility. So I think adding more extensibility regarding different tuple formats is an excellent thing to do.I'm going to mark it as RfC if there are no objections.
Thank you for your feedback. I also don't see how this patch could
affect anybody.
I'm going to push this if there are no objections.
I see this was pushed (cee1209514) so have closed it in the CF app.
Thanks
Ian Barwick
On Sat, Nov 26, 2022 at 2:05 PM Ian Lawrence Barwick <barwick@gmail.com> wrote:
2022年11月22日(火) 5:50 Alexander Korotkov <aekorotkov@gmail.com>:
On Mon, Nov 21, 2022 at 4:34 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not testedI've looked at this patch and don't see any problems with it. It is minimally invasive, it doesn't affect functionality unless anyone (e.g. extension) sets its own slotOps in CustomScanState.
Furthermore, the current patch very slightly modifies patch 0b03e5951bf0 with the intention of introducing extensibility. So I think adding more extensibility regarding different tuple formats is an excellent thing to do.I'm going to mark it as RfC if there are no objections.
Thank you for your feedback. I also don't see how this patch could
affect anybody.
I'm going to push this if there are no objections.I see this was pushed (cee1209514) so have closed it in the CF app.
Yes, I forgot to do this. Thank you.
------
Regards,
Alexander Korotkov