The const expression evaluation routine should always return a copy
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t49247psql -h localhost -U postgresBuilt from patchset v1 (message #1), September 13, 2026 at 04:19 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t49247_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t49247_1 && git checkout t49247_1Patchset v1 (message #1) is on t49247_1
IMO, the routine eval_const_expressions_mutator contains some stale code:
case T_SubPlan:
case T_AlternativeSubPlan:
/*
* Return a SubPlan unchanged --- too late to do anything with it.
*
* XXX should we ereport() here instead? Probably this routine
* should never be invoked after SubPlan creation.
*/
return node;
At least, this code could be achieved with estimate_expression_value().
So, we should fix the comment. But maybe we need to do a bit more.
According to the mutator idea, only the Query node is returned
unchanged. If the Subplan node is on top of the expression, the call
above returns the same node, which may be unconventional.
I'm not totally sure about the impossibility of constantifying SubPlan:
we already have InitPlans for uncorrelated subplans. Maybe something
about parameters that can be estimated as constants at this level and,
as a result, allow to return a Const instead of SubPlan?
But at least we can return a flat copy of the SubPplan node just for the
convention — the same thing for the AlternativeSubPlan. See the patch in
the attachment.
--
regards,
Andrei Lepikhov
Postgres Professional
Andrei Lepikhov <a.lepikhov@postgrespro.ru> writes:
IMO, the routine eval_const_expressions_mutator contains some stale code:
I'd like to push back against the idea that eval_const_expressions
is expected to return a freshly-copied tree. Its API specification
contains no such claim. It's true that it appears to do that
everywhere but here, but I think that's an implementation detail
that callers had better not depend on. It's not hard to imagine
someone trying to improve its performance by avoiding unnecessary
copying.
Also, your proposed patch introduces a great deal of schizophrenia,
because SubPlan has substructure. What's the point of making a copy
of the SubPlan node itself, if the testexpr and args aren't copied?
But we shouldn't modify those, because as the comment states, it's
a bit late to be doing so.
I agree that the comment claiming we can't get here is outdated,
but I'm unexcited about changing the code's behavior.
regards, tom lane
On 28/2/2024 04:19, Tom Lane wrote:
Andrei Lepikhov <a.lepikhov@postgrespro.ru> writes:
IMO, the routine eval_const_expressions_mutator contains some stale code:
I'd like to push back against the idea that eval_const_expressions
is expected to return a freshly-copied tree. Its API specification
contains no such claim. It's true that it appears to do that
everywhere but here, but I think that's an implementation detail
that callers had better not depend on. It's not hard to imagine
someone trying to improve its performance by avoiding unnecessary
copying.
Thanks for the explanation. I was just such a developer of extensions
who had looked into the internals of the eval_const_expressions, found a
call for a '_mutator' function, and made such a mistake :).
I agree that freeing the return node value doesn't provide a sensible
benefit because the underlying bushy tree was copied during mutation.
What's more, it makes even less sense with the selectivity context
coming shortly (I hope).
--
regards,
Andrei Lepikhov
Postgres Professional