From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Pavan Deolasee <pavan.deolasee@gmail.com>
Cc: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Cc: Peter Geoghegan <pg@bowt.ie>
Cc: Andres Freund <andres@anarazel.de>
Subject: Re: ON CONFLICT DO UPDATE for partitioned tables
Date: Fri, 16 Mar 2018 18:23:44 -0300
Message-ID: <20180316212344.e5hipioljibtx3xw@alvherre.pgsql> (raw)
In-Reply-To: <20180316151303.rml2p5wffn3o6qy6@alvherre.pgsql>
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.)
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:
typedef struct ResultRelInfo
{
...
/* for computing ON CONFLICT DO UPDATE SET */
TupleTableSlot *ri_onConflictProjSlot;
ProjectionInfo *ri_onConflictSetProj;
and with this the structure makes more sense. So ExecInitModifyTable
does this
/* create target slot for UPDATE SET projection */
tupDesc = ExecTypeFromTL((List *) node->onConflictSet,
relationDesc->tdhasoid);
resultRelInfo->ri_onConflictProjSlot =
ExecInitExtraTupleSlot(mtstate->ps.state, tupDesc);
/* build UPDATE SET projection state */
resultRelInfo->ri_onConflictSetProj =
ExecBuildProjectionInfo(node->onConflictSet, econtext,
resultRelInfo->ri_onConflictProjSlot,
&mtstate->ps, relationDesc);
and then ExecOnConflictUpdate can simply do this:
/* Project the new tuple version */
ExecProject(resultRelInfo->ri_onConflictSetProj);
/* Execute UPDATE with projection */
*returning = ExecUpdate(mtstate, &tuple.t_self, NULL,
resultRelInfo->ri_onConflictProjSlot, planSlot,
&mtstate->mt_epqstate, mtstate->ps.state,
canSetTag);
Now, maybe there is some reason I'm missing for the on conflict slot for
the projection to be in ModifyTableState rather than resultRelInfo. But
this code passes all current tests, so I don't know what that reason
would be.
Overall, the resulting code looks simpler to me than the previous
arrangements.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, pavan.deolasee@gmail.com, fujita.etsuro@lab.ntt.co.jp, Langote_Amit_f8@lab.ntt.co.jp, pg@bowt.ie, andres@anarazel.de
Subject: Re: ON CONFLICT DO UPDATE for partitioned tables
In-Reply-To: <20180316212344.e5hipioljibtx3xw@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox