Andres Freund wrote:
> Hi,
>
> On 2018-03-16 18:23:44 -0300, Alvaro Herrera wrote:
> > Another thing I noticed is that the split of the ON CONFLICT slot and
> > its corresponding projection is pretty weird. The projection is in
> > ResultRelInfo, but the slot itself is in ModifyTableState. You can't
> > make the projection work without a corresponding slot initialized with
> > the correct descriptor, so splitting it this way doesn't make a lot of
> > sense to me.
> >
> > (Now, TBH the split between resultRelInfo->ri_projectReturning and
> > ModifyTableState->ps.ps_ResultTupleSlot, which is the slot that the
> > returning project uses, doesn't make a lot of sense to me either; so
> > maybe there some reason that I'm just not seeing. But I digress.)
>
> The projections for different child tables / child plans can look
> different, therefore it can't be stored in ModifyTableState itself. No?
>
> The slot's descriptor is changed to be "appropriate" when necessary:
> if (slot->tts_tupleDescriptor != RelationGetDescr(resultRelationDesc))
> ExecSetSlotDescriptor(slot, RelationGetDescr(resultRelationDesc));
Grumble. This stuff looks like full of cheats to me, but I won't touch
it anyway.
> > So I want to propose that we move the slot to be together with the
> > projection node that it serves, ie. we put the slot in ResultRelInfo:
>
> That'll mean we have a good number of additional slots in some cases. I
> don't think the overhead of that is going to break the bank, but it's
> worth considering.
Good point.
I think what I should be doing is the same as the returning stuff: keep
a tupdesc around, and use a single slot, whose descriptor is changed
just before the projection.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
