Re: allow benign typedef redefinitions (C11)

On 2025-Sep-19, Álvaro Herrera wrote:

> I happened to realize that brin_tuple.h (a somewhat nasty header which
> was only supposed to be used by BRIN itself and stuff like amcheck)
> recently somehow snuck into tuplesort.h with some nefarious
> consequences: that one has polluted execnodes.h, which means that header
> now depends on struct definitions that appear in genam.h.  We should fix
> this.  This patch is not the full solution, just a way to show the
> problem; for a definitive solution, IMO the definitions of structs
> IndexScanInstrumentation and SharedIndexScanInstrumentation should be
> elsewhere so that execnodes.h doesn't have to include genam.h.
> 
> (gin_tuple.h is also there, but that one's much less of a problem.)

Here's a proposed fix for this issue, wherein I move the
IndexScanInstrumentation and SharedIndexScanInstrumentation struct
definitions from genam.h to a new file executor/instrument_node.h.
I think there's room to argue that we should also move
BitmapHeapScanInstrumentation and SharedBitmapHeapInstrumentation
(currently defined in execnodes.h) to the new file; any opinions on
this?  I think these structs belong together, so I'm leaning towards
yes.

Also, does anybody strongly dislike the proposed name?  I admit it looks
unlike the existing names there:

$ ls src/include/executor
execAsync.h            nodeFunctionscan.h         nodeSamplescan.h
execdebug.h            nodeGather.h               nodeSeqscan.h
execdesc.h             nodeGatherMerge.h          nodeSetOp.h
execExpr.h             nodeGroup.h                nodeSort.h
execParallel.h         nodeHash.h                 nodeSubplan.h
execPartition.h        nodeHashjoin.h             nodeSubqueryscan.h
execScan.h             nodeIncrementalSort.h      nodeTableFuncscan.h
executor.h             nodeIndexonlyscan.h        nodeTidrangescan.h
functions.h            nodeIndexscan.h            nodeTidscan.h
hashjoin.h             nodeLimit.h                nodeUnique.h
instrument.h           nodeLockRows.h             nodeValuesscan.h
instrument_node.h      nodeMaterial.h             nodeWindowAgg.h
nodeAgg.h              nodeMemoize.h              nodeWorktablescan.h
nodeAppend.h           nodeMergeAppend.h          spi.h
nodeBitmapAnd.h        nodeMergejoin.h            spi_priv.h
nodeBitmapHeapscan.h   nodeModifyTable.h          tablefunc.h
nodeBitmapIndexscan.h  nodeNamedtuplestorescan.h  tqueue.h
nodeBitmapOr.h         nodeNestloop.h             tstoreReceiver.h
nodeCtescan.h          nodeProjectSet.h           tuptable.h
nodeCustom.h           nodeRecursiveunion.h
nodeForeignscan.h      nodeResult.h

It's not the *first* name lacking camelCase or with an underscore, but
almost.  Note that there's already an instrument.h, which has a
completely unrelated charter.

I also left an XXX comment on genam.h about including instrument_node.h
there or adding the typedefs.  I think I prefer the typedefs myself.

CC'ing Peter G because of 0fbceae841cb and David because of
5a1e6df3b84c.

Thanks,

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Small aircraft do not crash frequently ... usually only once!"
                                  (ponder, http://thedailywtf.com/)