Hi, On 2026-Mar-13, Andres Freund wrote:
> Kinda wonder if funcapi.h should export tuplestore.h. That'd avoid needing > adding the dedicated tuplestore.h in so many places, and as the use of > tuplestore is basically required for funcapi.h users, it seems like it'd be > fine semantically? Hmm. I think there are plenty of SRF functions that use the value-per-call mechanics instead of a tuplestore (which don't need tuplestore.h), but I wouldn't be opposed to doing that. I was going to complain about that change dragging tuptable.h into funcapi.h -- until I realized that funcapi.h already includes tuptable.h directly itself. So I don't see any downsides. > If we could go back in time I'd wrap the tuplestore_puttuple() for the SRF use > case into an SRF specific wrapper, but my time travel capabilities have not > developed, despite no lack of trying. hah! (We could still change this now, and while it wouldn't change older code or existing third party extensions, it might definitely make things better going forward; the future being longer than the past, this might be good anyhow. But that's a matter for a separate thread.) > The need for dsa.h and condition_variable.h just is from > ParallelBitmapHeapState - which isn't actually an executor node and never > needed outside of nodeBitmapHeapscan.c - so it seems better to move it there? > > Added a commit for that. Whoa, nice! > Your patch numbering says 5/6, but there's only 5 attached, I assume that was > intentional? Yeah, the last one was about removing tidbitmap.h from genam.h -- I left it out at the last minute because it's unrelated to execnodes.h. Attached here. > I couldn't help myself to slim down execnodes.h further. Not sure if all of > them are quite worth it. The relpath.h one is definitely a good win. Not sure about stringinfo.h, which doesn't change very often and doesn't include anything else. I'm doubtful about the bitmapset.h removal gaining us much either (because the really nasty one below, nodetags.h, is unavoidable anyway.) > With all the commits combined very little low-level stuff is still > included. The worst is probably instr_time.h. Hm, that one is worth thinking about. But with only this much, it's already a huge win. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "No nos atrevemos a muchas cosas porque son difíciles, pero son difíciles porque no nos atrevemos a hacerlas" (Séneca)
>From 8e3349a2bbc5dd3704a143fccc977be18ad02476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]> Date: Fri, 6 Mar 2026 13:36:44 +0100 Subject: [PATCH 6/6] remove nodes/tidbitmap.h from genam.h --- src/backend/executor/nodeBitmapIndexscan.c | 1 + src/backend/optimizer/path/costsize.c | 1 + src/include/access/genam.h | 4 ++-- src/include/access/gin_private.h | 1 + 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/backend/executor/nodeBitmapIndexscan.c b/src/backend/executor/nodeBitmapIndexscan.c index 058a59ef5e7..b3838ab60ef 100644 --- a/src/backend/executor/nodeBitmapIndexscan.c +++ b/src/backend/executor/nodeBitmapIndexscan.c @@ -26,6 +26,7 @@ #include "executor/nodeBitmapIndexscan.h" #include "executor/nodeIndexscan.h" #include "miscadmin.h" +#include "nodes/tidbitmap.h" /* ---------------------------------------------------------------- diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index 89ca4e08bf1..56d45287c89 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -95,6 +95,7 @@ #include "miscadmin.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" +#include "nodes/tidbitmap.h" #include "optimizer/clauses.h" #include "optimizer/cost.h" #include "optimizer/optimizer.h" diff --git a/src/include/access/genam.h b/src/include/access/genam.h index 4c0429cc613..7b59cc44fde 100644 --- a/src/include/access/genam.h +++ b/src/include/access/genam.h @@ -18,7 +18,6 @@ #include "access/sdir.h" #include "access/skey.h" #include "executor/instrument_node.h" -#include "nodes/tidbitmap.h" #include "storage/buf.h" #include "storage/lockdefs.h" #include "utils/snapshot.h" @@ -26,7 +25,8 @@ /* We don't want this file to depend on execnodes.h. */ typedef struct IndexInfo IndexInfo; typedef struct TupleTableSlot TupleTableSlot; - +/* or tidbitmap.h */ +typedef struct TIDBitmap TIDBitmap; /* or relcache.h */ typedef struct RelationData *Relation; diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h index 7c3b4db94cd..4445d088fa0 100644 --- a/src/include/access/gin_private.h +++ b/src/include/access/gin_private.h @@ -19,6 +19,7 @@ #include "catalog/pg_am_d.h" #include "fmgr.h" #include "lib/rbtree.h" +#include "nodes/tidbitmap.h" #include "storage/bufmgr.h" /* -- 2.47.3
