From 08ea7a2291db21a618d19d612c8060cda68f1892 Mon Sep 17 00:00:00 2001 From: Simon Riggs Date: Thu, 12 Apr 2018 11:22:56 +0100 Subject: Revert MERGE patch This reverts commits d204ef63776b8a00ca220adec23979091564e465, 83454e3c2b28141c0db01c7d2027e01040df5249 and a few more commits thereafter (complete list at the end) related to MERGE feature. While the feature was fully functional, with sufficient test coverage and necessary documentation, it was felt that some parts of the executor and parse-analyzer can use a different design and it wasn't possible to do that in the available time. So it was decided to revert the patch for PG11 and retry again in the future. Thanks again to all reviewers and bug reporters. List of commits reverted, in reverse chronological order: f1464c5380 Improve parse representation for MERGE ddb4158579 MERGE syntax diagram correction 530e69e59b Allow cpluspluscheck to pass by renaming variable 01b88b4df5 MERGE minor errata 3af7b2b0d4 MERGE fix variable warning in non-assert builds a5d86181ec MERGE INSERT allows only one VALUES clause 4b2d44031f MERGE post-commit review 4923550c20 Tab completion for MERGE aa3faa3c7a WITH support in MERGE 83454e3c2b New files for MERGE d204ef6377 MERGE SQL Command following SQL:2016 Author: Pavan Deolasee Reviewed-by: Michael Paquier --- src/backend/executor/nodeModifyTable.c | 280 ++++++--------------------------- 1 file changed, 45 insertions(+), 235 deletions(-) (limited to 'src/backend/executor/nodeModifyTable.c') diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 543a735be2b..7ec2c6bcaa8 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -42,7 +42,6 @@ #include "commands/trigger.h" #include "executor/execPartition.h" #include "executor/executor.h" -#include "executor/execMerge.h" #include "executor/nodeModifyTable.h" #include "foreign/fdwapi.h" #include "miscadmin.h" @@ -63,6 +62,11 @@ static bool ExecOnConflictUpdate(ModifyTableState *mtstate, EState *estate, bool canSetTag, TupleTableSlot **returning); +static TupleTableSlot *ExecPrepareTupleRouting(ModifyTableState *mtstate, + EState *estate, + PartitionTupleRouting *proute, + ResultRelInfo *targetRelInfo, + TupleTableSlot *slot); static ResultRelInfo *getTargetResultRelInfo(ModifyTableState *node); static void ExecSetupChildParentMapForTcs(ModifyTableState *mtstate); static void ExecSetupChildParentMapForSubplan(ModifyTableState *mtstate); @@ -81,7 +85,7 @@ static TupleConversionMap *tupconv_map_for_subplan(ModifyTableState *node, * The plan output is represented by its targetlist, because that makes * handling the dropped-column case easier. */ -void +static void ExecCheckPlanOutput(Relation resultRel, List *targetList) { TupleDesc resultDesc = RelationGetDescr(resultRel); @@ -255,12 +259,11 @@ ExecCheckTIDVisible(EState *estate, * Returns RETURNING result if any, otherwise NULL. * ---------------------------------------------------------------- */ -extern TupleTableSlot * +static TupleTableSlot * ExecInsert(ModifyTableState *mtstate, TupleTableSlot *slot, TupleTableSlot *planSlot, EState *estate, - MergeActionState *actionState, bool canSetTag) { HeapTuple tuple; @@ -387,17 +390,9 @@ ExecInsert(ModifyTableState *mtstate, * partition, we should instead check UPDATE policies, because we are * executing policies defined on the target table, and not those * defined on the child partitions. - * - * If we're running MERGE, we refer to the action that we're executing - * to know if we're doing an INSERT or UPDATE to a partition table. */ - if (mtstate->operation == CMD_UPDATE) - wco_kind = WCO_RLS_UPDATE_CHECK; - else if (mtstate->operation == CMD_MERGE) - wco_kind = (actionState->commandType == CMD_UPDATE) ? - WCO_RLS_UPDATE_CHECK : WCO_RLS_INSERT_CHECK; - else - wco_kind = WCO_RLS_INSERT_CHECK; + wco_kind = (mtstate->operation == CMD_UPDATE) ? + WCO_RLS_UPDATE_CHECK : WCO_RLS_INSERT_CHECK; /* * ExecWithCheckOptions() will skip any WCOs which are not of the kind @@ -622,19 +617,10 @@ ExecInsert(ModifyTableState *mtstate, * passed to foreign table triggers; it is NULL when the foreign * table has no relevant triggers. * - * MERGE passes actionState of the action it's currently executing; - * regular DELETE passes NULL. This is used by ExecDelete to know if it's - * being called from MERGE or regular DELETE operation. - * - * If the DELETE fails because the tuple is concurrently updated/deleted - * by this or some other transaction, hufdp is filled with the reason as - * well as other important information. Currently only MERGE needs this - * information. - * * Returns RETURNING result if any, otherwise NULL. * ---------------------------------------------------------------- */ -TupleTableSlot * +static TupleTableSlot * ExecDelete(ModifyTableState *mtstate, ItemPointer tupleid, HeapTuple oldtuple, @@ -643,8 +629,6 @@ ExecDelete(ModifyTableState *mtstate, EState *estate, bool *tupleDeleted, bool processReturning, - HeapUpdateFailureData *hufdp, - MergeActionState *actionState, bool canSetTag, bool changingPart) { @@ -658,14 +642,6 @@ ExecDelete(ModifyTableState *mtstate, if (tupleDeleted) *tupleDeleted = false; - /* - * Initialize hufdp. Since the caller is only interested in the failure - * status, initialize with the state that is used to indicate successful - * operation. - */ - if (hufdp) - hufdp->result = HeapTupleMayBeUpdated; - /* * get information on the (current) result relation */ @@ -679,7 +655,7 @@ ExecDelete(ModifyTableState *mtstate, bool dodelete; dodelete = ExecBRDeleteTriggers(estate, epqstate, resultRelInfo, - tupleid, oldtuple, hufdp); + tupleid, oldtuple); if (!dodelete) /* "do nothing" */ return NULL; @@ -747,15 +723,6 @@ ldelete:; true /* wait for commit */ , &hufd, changingPart); - - /* - * Copy the necessary information, if the caller has asked for it. We - * must do this irrespective of whether the tuple was updated or - * deleted. - */ - if (hufdp) - *hufdp = hufd; - switch (result) { case HeapTupleSelfUpdated: @@ -790,11 +757,7 @@ ldelete:; errmsg("tuple to be updated was already modified by an operation triggered by the current command"), errhint("Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows."))); - /* - * Else, already deleted by self; nothing to do but inform - * MERGE about it anyways so that it can take necessary - * action. - */ + /* Else, already deleted by self; nothing to do */ return NULL; case HeapTupleMayBeUpdated: @@ -814,19 +777,10 @@ ldelete:; { TupleTableSlot *epqslot; - /* - * If we're executing MERGE, then the onus of running - * EvalPlanQual() and handling its outcome lies with the - * caller. - */ - if (actionState != NULL) - return NULL; - - /* Normal DELETE path. */ epqslot = EvalPlanQual(estate, epqstate, resultRelationDesc, - GetEPQRangeTableIndex(resultRelInfo), + resultRelInfo->ri_RangeTableIndex, LockTupleExclusive, &hufd.ctid, hufd.xmax); @@ -836,12 +790,7 @@ ldelete:; goto ldelete; } } - - /* - * tuple already deleted; nothing to do. But MERGE might want - * to handle it differently. We've already filled-in hufdp - * with sufficient information for MERGE to look at. - */ + /* tuple already deleted; nothing to do */ return NULL; default: @@ -969,21 +918,10 @@ ldelete:; * foreign table triggers; it is NULL when the foreign table has * no relevant triggers. * - * MERGE passes actionState of the action it's currently executing; - * regular UPDATE passes NULL. This is used by ExecUpdate to know if it's - * being called from MERGE or regular UPDATE operation. ExecUpdate may - * pass this information to ExecInsert if it ends up running DELETE+INSERT - * for partition key updates. - * - * If the UPDATE fails because the tuple is concurrently updated/deleted - * by this or some other transaction, hufdp is filled with the reason as - * well as other important information. Currently only MERGE needs this - * information. - * * Returns RETURNING result if any, otherwise NULL. * ---------------------------------------------------------------- */ -extern TupleTableSlot * +static TupleTableSlot * ExecUpdate(ModifyTableState *mtstate, ItemPointer tupleid, HeapTuple oldtuple, @@ -991,9 +929,6 @@ ExecUpdate(ModifyTableState *mtstate, TupleTableSlot *planSlot, EPQState *epqstate, EState *estate, - bool *tuple_updated, - HeapUpdateFailureData *hufdp, - MergeActionState *actionState, bool canSetTag) { HeapTuple tuple; @@ -1010,17 +945,6 @@ ExecUpdate(ModifyTableState *mtstate, if (IsBootstrapProcessingMode()) elog(ERROR, "cannot UPDATE during bootstrap"); - if (tuple_updated) - *tuple_updated = false; - - /* - * Initialize hufdp. Since the caller is only interested in the failure - * status, initialize with the state that is used to indicate successful - * operation. - */ - if (hufdp) - hufdp->result = HeapTupleMayBeUpdated; - /* * get the heap tuple out of the tuple table slot, making sure we have a * writable copy @@ -1038,7 +962,7 @@ ExecUpdate(ModifyTableState *mtstate, resultRelInfo->ri_TrigDesc->trig_update_before_row) { slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo, - tupleid, oldtuple, slot, hufdp); + tupleid, oldtuple, slot); if (slot == NULL) /* "do nothing" */ return NULL; @@ -1084,6 +1008,7 @@ ExecUpdate(ModifyTableState *mtstate, } else { + LockTupleMode lockmode; bool partition_constraint_failed; /* @@ -1162,7 +1087,7 @@ lreplace:; * processing. We want to return rows from INSERT. */ ExecDelete(mtstate, tupleid, oldtuple, planSlot, epqstate, - estate, &tuple_deleted, false, hufdp, NULL, + estate, &tuple_deleted, false, false /* canSetTag */, true /* changingPart */); /* @@ -1199,36 +1124,16 @@ lreplace:; saved_tcs_map = mtstate->mt_transition_capture->tcs_map; /* - * We should convert the tuple into root's tuple descriptor, since - * ExecInsert() starts the search from root. To do that, we need to - * retrieve the tuple conversion map for this resultRelInfo. - * - * If we're running MERGE then resultRelInfo is per-partition - * resultRelInfo as initialized in ExecInitPartitionInfo(). Note - * that we don't expand inheritance for the resultRelation in case - * of MERGE and hence there is just one subplan. Whereas for - * regular UPDATE, resultRelInfo is one of the per-subplan - * resultRelInfos. In either case the position of this partition in - * tracked in ri_PartitionLeafIndex; - * - * Retrieve the map either by looking at the resultRelInfo's - * position in mtstate->resultRelInfo[] (for UPDATE) or by simply - * using the ri_PartitionLeafIndex value (for MERGE). + * resultRelInfo is one of the per-subplan resultRelInfos. So we + * should convert the tuple into root's tuple descriptor, since + * ExecInsert() starts the search from root. The tuple conversion + * map list is in the order of mtstate->resultRelInfo[], so to + * retrieve the one for this resultRel, we need to know the + * position of the resultRel in mtstate->resultRelInfo[]. */ - if (mtstate->operation == CMD_MERGE) - { - map_index = resultRelInfo->ri_PartitionLeafIndex; - Assert(mtstate->rootResultRelInfo == NULL); - tupconv_map = TupConvMapForLeaf(proute, - mtstate->resultRelInfo, - map_index); - } - else - { - map_index = resultRelInfo - mtstate->resultRelInfo; - Assert(map_index >= 0 && map_index < mtstate->mt_nplans); - tupconv_map = tupconv_map_for_subplan(mtstate, map_index); - } + map_index = resultRelInfo - mtstate->resultRelInfo; + Assert(map_index >= 0 && map_index < mtstate->mt_nplans); + tupconv_map = tupconv_map_for_subplan(mtstate, map_index); tuple = ConvertPartitionTupleSlot(tupconv_map, tuple, proute->root_tuple_slot, @@ -1238,16 +1143,12 @@ lreplace:; * Prepare for tuple routing, making it look like we're inserting * into the root. */ + Assert(mtstate->rootResultRelInfo != NULL); slot = ExecPrepareTupleRouting(mtstate, estate, proute, - getTargetResultRelInfo(mtstate), - slot); + mtstate->rootResultRelInfo, slot); ret_slot = ExecInsert(mtstate, slot, planSlot, - estate, actionState, canSetTag); - - /* Update is successful. */ - if (tuple_updated) - *tuple_updated = true; + estate, canSetTag); /* Revert ExecPrepareTupleRouting's node change. */ estate->es_result_relation_info = resultRelInfo; @@ -1285,16 +1186,7 @@ lreplace:; estate->es_output_cid, estate->es_crosscheck_snapshot, true /* wait for commit */ , - &hufd); - - /* - * Copy the necessary information, if the caller has asked for it. We - * must do this irrespective of whether the tuple was updated or - * deleted. - */ - if (hufdp) - *hufdp = hufd; - + &hufd, &lockmode); switch (result) { case HeapTupleSelfUpdated: @@ -1348,37 +1240,22 @@ lreplace:; { TupleTableSlot *epqslot; - /* - * If we're executing MERGE, then the onus of running - * EvalPlanQual() and handling its outcome lies with the - * caller. - */ - if (actionState != NULL) - return NULL; - - /* Regular UPDATE path. */ epqslot = EvalPlanQual(estate, epqstate, resultRelationDesc, - GetEPQRangeTableIndex(resultRelInfo), - hufd.lockmode, + resultRelInfo->ri_RangeTableIndex, + lockmode, &hufd.ctid, hufd.xmax); if (!TupIsNull(epqslot)) { *tupleid = hufd.ctid; - /* Normal UPDATE path */ slot = ExecFilterJunk(resultRelInfo->ri_junkFilter, epqslot); tuple = ExecMaterializeSlot(slot); goto lreplace; } } - - /* - * tuple already deleted; nothing to do. But MERGE might want - * to handle it differently. We've already filled-in hufdp - * with sufficient information for MERGE to look at. - */ + /* tuple already deleted; nothing to do */ return NULL; default: @@ -1407,9 +1284,6 @@ lreplace:; estate, false, NULL, NIL); } - if (tuple_updated) - *tuple_updated = true; - if (canSetTag) (estate->es_processed)++; @@ -1504,9 +1378,9 @@ ExecOnConflictUpdate(ModifyTableState *mtstate, * there's no historical behavior to break. * * It is the user's responsibility to prevent this situation from - * occurring. These problems are why SQL Standard similarly - * specifies that for SQL MERGE, an exception must be raised in - * the event of an attempt to update the same row twice. + * occurring. These problems are why SQL-2003 similarly specifies + * that for SQL MERGE, an exception must be raised in the event of + * an attempt to update the same row twice. */ if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple.t_data))) ereport(ERROR, @@ -1636,7 +1510,7 @@ ExecOnConflictUpdate(ModifyTableState *mtstate, *returning = ExecUpdate(mtstate, &tuple.t_self, NULL, mtstate->mt_conflproj, planSlot, &mtstate->mt_epqstate, mtstate->ps.state, - NULL, NULL, NULL, canSetTag); + canSetTag); ReleaseBuffer(buffer); return true; @@ -1674,14 +1548,6 @@ fireBSTriggers(ModifyTableState *node) case CMD_DELETE: ExecBSDeleteTriggers(node->ps.state, resultRelInfo); break; - case CMD_MERGE: - if (node->mt_merge_subcommands & MERGE_INSERT) - ExecBSInsertTriggers(node->ps.state, resultRelInfo); - if (node->mt_merge_subcommands & MERGE_UPDATE) - ExecBSUpdateTriggers(node->ps.state, resultRelInfo); - if (node->mt_merge_subcommands & MERGE_DELETE) - ExecBSDeleteTriggers(node->ps.state, resultRelInfo); - break; default: elog(ERROR, "unknown operation"); break; @@ -1737,17 +1603,6 @@ fireASTriggers(ModifyTableState *node) ExecASDeleteTriggers(node->ps.state, resultRelInfo, node->mt_transition_capture); break; - case CMD_MERGE: - if (node->mt_merge_subcommands & MERGE_DELETE) - ExecASDeleteTriggers(node->ps.state, resultRelInfo, - node->mt_transition_capture); - if (node->mt_merge_subcommands & MERGE_UPDATE) - ExecASUpdateTriggers(node->ps.state, resultRelInfo, - node->mt_transition_capture); - if (node->mt_merge_subcommands & MERGE_INSERT) - ExecASInsertTriggers(node->ps.state, resultRelInfo, - node->mt_transition_capture); - break; default: elog(ERROR, "unknown operation"); break; @@ -1810,7 +1665,7 @@ ExecSetupTransitionCaptureState(ModifyTableState *mtstate, EState *estate) * * Returns a slot holding the tuple of the partition rowtype. */ -TupleTableSlot * +static TupleTableSlot * ExecPrepareTupleRouting(ModifyTableState *mtstate, EState *estate, PartitionTupleRouting *proute, @@ -2143,7 +1998,6 @@ ExecModifyTable(PlanState *pstate) { /* advance to next subplan if any */ node->mt_whichplan++; - if (node->mt_whichplan < node->mt_nplans) { resultRelInfo++; @@ -2192,12 +2046,6 @@ ExecModifyTable(PlanState *pstate) EvalPlanQualSetSlot(&node->mt_epqstate, planSlot); slot = planSlot; - if (operation == CMD_MERGE) - { - ExecMerge(node, estate, slot, junkfilter, resultRelInfo); - continue; - } - tupleid = NULL; oldtuple = NULL; if (junkfilter != NULL) @@ -2279,20 +2127,19 @@ ExecModifyTable(PlanState *pstate) slot = ExecPrepareTupleRouting(node, estate, proute, resultRelInfo, slot); slot = ExecInsert(node, slot, planSlot, - estate, NULL, node->canSetTag); + estate, node->canSetTag); /* Revert ExecPrepareTupleRouting's state change. */ if (proute) estate->es_result_relation_info = resultRelInfo; break; case CMD_UPDATE: slot = ExecUpdate(node, tupleid, oldtuple, slot, planSlot, - &node->mt_epqstate, estate, - NULL, NULL, NULL, node->canSetTag); + &node->mt_epqstate, estate, node->canSetTag); break; case CMD_DELETE: slot = ExecDelete(node, tupleid, oldtuple, planSlot, &node->mt_epqstate, estate, - NULL, true, NULL, NULL, node->canSetTag, + NULL, true, node->canSetTag, false /* changingPart */); break; default: @@ -2383,16 +2230,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) saved_resultRelInfo = estate->es_result_relation_info; resultRelInfo = mtstate->resultRelInfo; - - /* - * mergeTargetRelation must be set if we're running MERGE and mustn't be - * set if we're not. - */ - Assert(operation != CMD_MERGE || node->mergeTargetRelation > 0); - Assert(operation == CMD_MERGE || node->mergeTargetRelation == 0); - - resultRelInfo->ri_mergeTargetRTI = node->mergeTargetRelation; - i = 0; foreach(l, node->plans) { @@ -2471,8 +2308,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) * partition key. */ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && - (operation == CMD_INSERT || operation == CMD_MERGE || - update_tuple_routing_needed)) + (operation == CMD_INSERT || update_tuple_routing_needed)) mtstate->mt_partition_tuple_routing = ExecSetupPartitionTupleRouting(mtstate, rel); @@ -2483,15 +2319,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) if (!(eflags & EXEC_FLAG_EXPLAIN_ONLY)) ExecSetupTransitionCaptureState(mtstate, estate); - /* - * If we are doing MERGE then setup child-parent mapping. This will be - * required in case we end up doing a partition-key update, triggering a - * tuple routing. - */ - if (mtstate->operation == CMD_MERGE && - mtstate->mt_partition_tuple_routing != NULL) - ExecSetupChildParentMapForLeaf(mtstate->mt_partition_tuple_routing); - /* * Construct mapping from each of the per-subplan partition attnos to the * root attno. This is required when during update row movement the tuple @@ -2684,10 +2511,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) } } - resultRelInfo = mtstate->resultRelInfo; - if (mtstate->operation == CMD_MERGE) - ExecInitMerge(mtstate, estate, resultRelInfo); - /* select first subplan */ mtstate->mt_whichplan = 0; subplan = (Plan *) linitial(node->plans); @@ -2701,7 +2524,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) * --- no need to look first. Typically, this will be a 'ctid' or * 'wholerow' attribute, but in the case of a foreign data wrapper it * might be a set of junk attributes sufficient to identify the remote - * row. We follow this logic for MERGE, so it always has a junk attributes. + * row. * * If there are multiple result relations, each one needs its own junk * filter. Note multiple rels are only possible for UPDATE/DELETE, so we @@ -2729,7 +2552,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) break; case CMD_UPDATE: case CMD_DELETE: - case CMD_MERGE: junk_filter_needed = true; break; default: @@ -2745,7 +2567,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) JunkFilter *j; subplan = mtstate->mt_plans[i]->plan; - if (operation == CMD_INSERT || operation == CMD_UPDATE) ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc, subplan->targetlist); @@ -2754,9 +2575,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) resultRelInfo->ri_RelationDesc->rd_att->tdhasoid, ExecInitExtraTupleSlot(estate, NULL)); - if (operation == CMD_UPDATE || - operation == CMD_DELETE || - operation == CMD_MERGE) + if (operation == CMD_UPDATE || operation == CMD_DELETE) { /* For UPDATE/DELETE, find the appropriate junk attr now */ char relkind; @@ -2769,15 +2588,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) j->jf_junkAttNo = ExecFindJunkAttribute(j, "ctid"); if (!AttributeNumberIsValid(j->jf_junkAttNo)) elog(ERROR, "could not find junk ctid column"); - - if (operation == CMD_MERGE && - relkind == RELKIND_PARTITIONED_TABLE) - { - j->jf_otherJunkAttNo = ExecFindJunkAttribute(j, "tableoid"); - if (!AttributeNumberIsValid(j->jf_otherJunkAttNo)) - elog(ERROR, "could not find junk tableoid column"); - - } } else if (relkind == RELKIND_FOREIGN_TABLE) { -- cgit v1.2.3