On Wed, Mar 9, 2022 at 9:38 AM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
> I attach MERGE v14. This includes a fix from Amit Langote for the > problem I described previously, with EvalPlanQual not working correctly. > (I had failed to short-circuit the cross-partition update correctly.) > Now the test case is enabled again, and it passes with the correct > output. > > 0001 is as before; the only change is that I've added a commit message. > Since this just moves some code around, I intend to get it pushed very > soon. > > 0002 is the ExecUpdate/ExecDelete split. I cleaned up the struct > definitions a bit more, but it's pretty much as in the previous version. > > 0003 is the actual MERGE implementation. Many previous review comments > have been handled, and several other things are cleaner than before. > I haven't made any changes for work_mem handling in ModifyTable's > transition tables, though, and haven't attempted to address concurrent > INSERT. > > -- > Álvaro Herrera 39°49'30"S 73°17'W — > https://www.EnterpriseDB.com/ Hi, For v14-0002-Split-ExecUpdate-and-ExecDelete-in-reusable-piec.patch : + TupleTableSlot *insertProjectedTuple; With `insert` at the beginning of the variable name, someone may think it is a verb. How about naming the variable projectedTupleFromInsert (or something similar) ? + if (!ExecBRDeleteTriggers(context->estate, context->epqstate, + resultRelInfo, tupleid, oldtuple, + epqreturnslot)) + /* some trigger made the delete a no-op; let caller know */ + return false; It seems you can directly return the value returned from ExecBRDeleteTriggers(). + if (!ExecBRUpdateTriggers(context->estate, context->epqstate, + resultRelInfo, tupleid, oldtuple, slot)) + /* some trigger made the update a no-op; let caller know */ + return false; Similar comment as above (the comment is good and should be kept). Cheers
