Re: BUG #17800: ON CONFLICT DO UPDATE fails to detect incompatible fields that leads to a server crash

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2023-02-21 15:16:11 -0500, Tom Lane wrote:
>> It occurs to me that one possible fix is to make MULTIEXPR_SUBLINK
>> and the associated output Params use a separate ParamExecData array;
>> instead of the query-wide es_param_exec_vals array, use one that
>> is local to the specific targetlist's ExprState. I'm not sure how
>> much violence that does to the current notion of an ExprState ---
>> do we think that is read-only during execution?

> I don't think you would need to modify ExprState - the information about
> params etc comes from the ExprContext, right? So we'd need to build a
> different ExprContext for partitions, and use that when evaluating the
> expressions.
> ...
> We don't currently have infrastructure for setting
> econtext->ecxt_param_exec_vals to something else, but that shouldn't be too
> hard to add.

No, that won't work, because many usages of PARAM_EXEC Params are
specifically intended to transmit datums from one expression (plan node)
to another. That's why that array was query-global to begin with.
What I'm wondering about is adding a separate array, and likely a separate
ParamKind, that would have a less-than-query-wide scope. We might be able
to get away with having that be plan-node-wide, but making it local to the
specific compiled expression feels safer and easier to reason about.

>> If we did have a local-to-the-expression ParamExecData array, maybe that
>> could be used to get a cleaner fix for things like the domain VALUES
>> and case-test-expression hacks.

> Hm, I'm not quite following along here.

I'm just arm-waving at this point, it's not real clear to me either.
But I do remember that we have some ugly hacks centered around the
fact that domain VALUES and CaseTestExpr are implemented with a single
datum slot per EContext. I'd rather convert them into something like
PARAM_EXEC with no sharing.

regards, tom lane