> On Fri, Oct 27, 2023 at 05:02:44PM +0200, Dmitry Dolgov wrote:
> > On Thu, Oct 26, 2023 at 09:08:42AM +0900, Michael Paquier wrote:
> > typedef struct ArrayExpr
> > {
> > + pg_node_attr(custom_query_jumble)
> > +
> >
> > Hmm. I am not sure that this is the best approach
> > implementation-wise. Wouldn't it be better to invent a new
> > pg_node_attr (these can include parameters as well!), say
> > query_jumble_merge or query_jumble_agg_location that aggregates all
> > the parameters of a list to be considered as a single element. To put
> > it short, we could also apply the same property to other parts of a
> > parsed tree, and not only an ArrayExpr's list.
>
> Sounds like an interesting idea, something like:
>
> typedef struct ArrayExpr
> {
> ...
> List *elements pg_node_attr(query_jumble_merge);
>
> to replace simple JUMBLE_NODE(elements) with more elaborated logic.
>
> > /* GUC parameters */
> > extern PGDLLIMPORT int compute_query_id;
> > -
> > +extern PGDLLIMPORT bool query_id_const_merge;
> >
> > Not much a fan of this addition as well for an in-core GUC. I would
> > suggest pushing the GUC layer to pg_stat_statements, maintaining the
> > computation method to use as a field of JumbleState as I suspect that
> > this is something we should not enforce system-wide, but at
> > extension-level instead.
>
> I also do not particularly like an extra GUC here, but as far as I can
> tell to make it pg_stat_statements GUC only it has to be something
> similar to EnableQueryId (e.g. EnableQueryConstMerging), that will be
> called from pgss. Does this sound better?
For clarity, here is what I had in mind for those two points.