> > I do see the discussion here [1], sorry for not noticing it. > > > > I am not sure about this though. At minimum this needs to be documented, > > However, I think the prepared statement case is too common of a case to > > skip for the first release of tis feature, and users that will likely > > benefit from this feature are using prepared statements ( i.e. JDBC, etc ). > > Right, prepared statements are quite common case. This would be the > first thing I'll take on in the case if this patch will find it's way > into the release. As you can see it's not at all obvious that that will > happen, I estimate chances for that to be higher if moving in smaller > steps.
I think it will be sad to not include this very common case from the start, because it is going to be one of the most common cases. Wouldn't doing something like this inside IsMergeableConst """ if (!IsA(arg, Const) && !IsA(arg, Param)) """ instead of """ if (!IsA(arg, Const)) """ be sufficient? > > I can't think of other cases beyond ArrayExpr where this will be needed. > > The node that could use this will need to carry constants, but ArrayExpr > > is the only case I can think of in which this will be useful for jumbling. > > There should be a really good reason IMO to do something other than the > > existing pattern of using custom_query_jumble. > > Well, there are plenty expression nodes that have lists in them, maybe > more will be added in the future. And as before, the idea of using > pg_node_attr was a resonable suggestion from Michael Paquier on top of > the original design (which indeed used custom jumble function for > ArrayExpr). OK. I don't necessarily agree with this, but it's been discussed [1] and I will not push this point any further. > > It's not a functionality regression as far as query execution > > or pg_stat_statements counters go, but it is a regression as far as > > displaying query text in pg_stat_statements. pg_stat_statements, unlike > > pg_stat_acitivty, makes a guaranteee not to trim text as stated in the docs > > [2] > > "The representative query texts are kept in an external disk file, > > and do not consume shared memory. Therefore, > > even very lengthy query texts can be stored successfully." > > Just to clarify, the part you reference doesn't say anything about > trimming, doesn't it? In fact, the query text stored in > pg_stat_statements might be as well somewhat different from one that was > executed, due to similar queries having the same query_id and differ > only in e.g. parenthesis. I perhap meant "missing chunk" instead of "trimming". To me it just looked like a trimmed text, which was wrong. Looks like v25 deals with that better at least. I am just not sure about all that we are doing here as I believe it may open up big changes for bugs generating the normalized query texts. I'm a bit worried about that. IMO, we are better off just adding a comment at the start of a query that this query text such as "/* query_id_squash_values */" and keeping all the parameter symbols in-place. [1] https://www.postgresql.org/message-id/ZTmuCtymIS3n3fP_%40paquier.xyz -- Sami
