On Tue, Oct 19, 2021 at 10:04 AM Tom Lane <t...@sss.pgh.pa.us> wrote:
> [ please do not quote the entire thread when replying ]
>
> Zhihong Yu <z...@yugabyte.com> writes:
> > Here is the patch.
>
> This patch seems quite misguided to me. The proximate cause of
> the crash is that we're arriving at ExecEvalFieldStoreDeForm with
> *op->resnull and *op->resvalue both zero, which is a completely
> invalid situation for a pass-by-reference datatype; so something
> upstream of this messed up. Even if there were an argument for
> acting as though that were a valid NULL value, this patch fails to
> do so; that'd require setting all the output fieldstore.nulls[]
> entries to true, which you didn't.
>
> Moreover, experiment quickly shows that the problem only shows up with
> an array of domain over composite, not an array of plain composite.
> The proposed patch doesn't seem to have anything to do with that
> observation.
>
> After some digging around, I see where the issue actually is:
> the expression tree we're dealing with looks like
>
> {SUBSCRIPTINGREF
> :refexpr
> {VAR
> }
> :refassgnexpr
> {COERCETODOMAIN
> :arg
> {FIELDSTORE
> :arg
> {CASETESTEXPR
> }
> }
> }
> }
>
> The array element we intend to replace has to be passed down to
> the CaseTestExpr, but that isn't happening. That's because
> isAssignmentIndirectionExpr fails to recognize a tree like
> this, so ExecInitSubscriptingRef doesn't realize it needs to
> arrange for that.
>
> I believe the attached is a correct fix.
>
> regards, tom lane
>
> Hi,
Tom's patch fixes the update of individual field inside the domain type as
well.
Tom's patch looks good to me.
