Date: Fri, 16 Mar 2018 18:46:17 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Andres Freund <andres@anarazel.de>
Cc: Pavan Deolasee <pavan.deolasee@gmail.com>,
	Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>,
	Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>,
	Pg Hackers <pgsql-hackers@postgresql.org>,
	Peter Geoghegan <pg@bowt.ie>
Subject: Re: ON CONFLICT DO UPDATE for partitioned tables
Message-ID: <20180316214617.dpunmkhhjhvgfxpi@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180316212851.erj4dwmz3ubq3kiq@alap3.anarazel.de>
User-Agent: NeoMutt/20170306-137-4415bd-dirty (1.8.0)
Precedence: bulk

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