On 2018/02/16 12:41, Etsuro Fujita wrote:
> (2018/02/16 10:49), Amit Langote wrote:
>> I think you're right. If node->returningLists is non-NULL at all,
>> ExecInitModifyTable() would've initialized the needed slot and expression
>> context. I added Assert()s to that affect.
>
> OK, but one thing I'd like to ask is:
>
> + /*
> + * Use the slot that would have been set up in ExecInitModifyTable()
> + * for the output of the RETURNING projection(s). Just make sure to
> + * assign its rowtype using the RETURNING list.
> + */
> + Assert(mtstate->ps.ps_ResultTupleSlot != NULL);
> + tupDesc = ExecTypeFromTL(returningList, false);
> + ExecAssignResultType(&mtstate->ps, tupDesc);
> + slot = mtstate->ps.ps_ResultTupleSlot;
>
> Do we need that assignment here?
I guess mean the assignment of rowtype, that is, the
ExecAssignResultType() line. On looking at this some more, it looks like
we don't need to ExecAssignResultType here, as you seem to be suspecting,
because we want the RETURNING projection output to use the rowtype of the
first of returningLists and that's what mtstate->ps.ps_ResultTupleSlot has
been set to use in the first place. So, removed the ExecAssignResultType().
Attached v9. Thanks a for the review!
Regards,
Amit