> On Tue, Feb 11, 2025 at 10:49:59AM GMT, Sami Imseih wrote:
> I have only looked at 0001, but I am wondering why
> query_id_const_merge is a pg_stat_statements GUC
> rather than a core GUC?
It was moved from being a core GUC into a pg_stat_statements GUC on the request
from the reviewers. Community tries to prevent adding more and more core GUCs
into PostgreSQL.
> On Tue, Feb 11, 2025 at 07:18:23PM GMT, Álvaro Herrera wrote:
> On 2025-Feb-11, Sami Imseih wrote:
>
> > I do not have an explanation from the patch yet, but I have a test
> > that appears to show unexpected results. I only tested a few datatypes,
> > but from what I observe, some merge as expected and others do not;
> > i.e. int columns merge correctly but bigint do not.
>
> Yep, I noticed this too, and realized that this is because these values
> are wrapped in casts of some sort, while the others are not.
>
> > select from foo where col_bigint in (1, 2, 3);
> > select from foo where col_bigint in (1, 2, 3, 4);
> > select from foo where col_bigint in (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
> > select from foo where col_float in (1, 2, 3);
> > select from foo where col_float in (1, 2, 3, 4);
>
> You can see that it works correctly if you use quotes around the values,
> e.g.
> select from foo where col_float in ('1', '2', '3');
> select from foo where col_float in ('1', '2', '3', '4');
> and so on. There are no casts here because these literals are of type
> unknown.
>
> I suppose this is telling us that detecting the case with consts wrapped
> in casts is not really optional. (Dmitry said this was supported at
> early stages of the patch, and now I'm really curious about that
> implementation because what IsMergeableConstList sees is a FuncExpr that
> invokes the cast function for float8 to int4.)
Yes, those cases in question are usually FuncExpr. The original patch
implementation used to handle that via simplifying the node with
eval_const_expressions to figure out if the value we work with is a constant.
This approach was marked as too risky by reviewers, as this could reach a lot
of unexpected functionality in the mutator part.
