Hi,
On 2026-01-06 11:40:01 -0500, Tom Lane wrote:
> I wrote:
> > After re-reading that patch, I suspect an aliasing problem,
> > specifically from this bit in ExecInitSubPlanExpr:
>
> > * ... No
> > * danger of conflicts with other uses of resvalue/resnull as storing and
> > * using the value always is in subsequent steps.
>
> > ExecInitExprRec(arg, state,
> > &state->resvalue, &state->resnull);
>
> Yup, that's it. The problem occurs when a SubPlan is in the
> subscripts of an array reference. In that case,
> ExecInitSubscriptingRef has already emitted code to load the source
> array into its target resv/resnull, which might well be the
> ExprState's resvalue/resnull.
I see we pinpointed the same thing...
> So it's not okay for the array subscript calculation steps to overwrite the
> ExprState's resvalue/resnull, but ExecInitSubPlanExpr thinks it can.
But I'm less sure that the problem is ExecInitSubPlanExpr()'s use of
state->resvalue, rather than ExecInitSubscriptingRef() belief that the *resv
value won't change.
If I could travel through time, I'd tell younger Andres to introduce explicit
variables to the expression interpretation thingymagick...
> We *could* safely use ExecInitSubPlanExpr's target resv/resnull,
> but that doesn't line up with EEOP_PARAM_SET's definition:
> ExecEvalParamSet is hard-wired to store from state->resvalue/resnull.
> I thought all along that that was probably too simplistic.
>
> We could either generalize EEOP_PARAM_SET to include an explicit
> specification of the source value's address
That's pretty trivial, see attached. I don't quite understand why I didn't go
that way immediately...
At the very least we need to create a simplified testcase for the bug at hand.
> or insert some kind of LOAD operation to copy the computed value into
> state->resvalue/resnull. I don't see anything that looks like that today,
> though.
Hm, wouldn't that have exactly the same issues as we have today anyway?
Greetings,
Andres Freund
