Re: [PATCH} Move instrumentation structs

On Mon, 15 Dec 2025 at 11:45, Mario González Troncoso
<gonzalemario(at)gmail(dot)com> wrote:
>
>
> Thanks guys for the feedback. I rebased from master as well and
> applied the suggestions.
> Regarding changing the comments, I reckon we can do it in the next
> iteration if you still consider it worth it. But the code looks good
> to me, I hope it does for you too:
> https://cirrus-ci.com/build/6530088079982592
>

Hey there. I'm updating the patch. BTW, only 0002 has changed where,
because of `213a1b89527 Move attribute statistics functions to
stat_utils.c`, we're also adding "typcache.h"
--- a/src/backend/statistics/stat_utils.c
+++ b/src/backend/statistics/stat_utils.c
@@ -35,6 +35,7 @@
#include "utils/lsyscache.h"
#include "utils/rel.h"
#include "utils/syscache.h"
+#include "utils/typcache.h"

https://cirrus-ci.com/build/6354305906638848

>
> > +/* ----------------
> > + * Values displayed by EXPLAIN ANALYZE
> > + * ----------------
> > + */
> > +typedef struct HashInstrumentation
> >
> > This should say something like "Instrumentation for Hash nodes" or
> > something like that. Less critically, the comment styling (those lines
> > of dashes, vertical spacing, and so on) should be made consistent across
> > the whole instrument_node.h file instead of using whatever was in the
> > original file, which is an eclectic mix of various different styles.
> >
> > Another thing I'd do here is make 0001 as minimal as possible. I see
> > that some files get a new #include "utils/typcache.h" line (or amapi.h
> > or genam.h), for instance, but that change makes no sense in that patch.
> > These additions should be in the 0002 patch. The only new #include line
> > in the 0001 patch should be instrument_node.h itself, because we're
> > explicitly not removing any other #include line anywhere.
> >
> >

--
Mario Gonzalez
EDB: https://www.enterprisedb.com