diff --git a/src/backend/executor/nodeDML.c b/src/backend/executor/nodeDML.c index 33b2edf5387f..c1e82ee7595c 100644 --- a/src/backend/executor/nodeDML.c +++ b/src/backend/executor/nodeDML.c @@ -57,20 +57,19 @@ ExecDML(DMLState *node) { return NULL; } + EvalPlanQualSetSlot(&node->mt_epqstate, slot); bool isnull = false; - int action = DatumGetUInt32(slot_getattr(slot, plannode->actionColIdx, &isnull)); - Assert(!isnull); + int action = -1; + bool isUpdate = node->ps.state->es_plannedstmt->commandType == CMD_UPDATE; - bool isUpdate = false; - if (node->ps.state->es_plannedstmt->commandType == CMD_UPDATE) + // if it's not in place update + if(plannode->actionColIdx) { - isUpdate = true; + action = DatumGetUInt32(slot_getattr(slot, plannode->actionColIdx, &isnull)); + Assert(!isnull); } - Assert(action == DML_INSERT || action == DML_DELETE); - - /* * Reset per-tuple memory context to free any expression evaluation * storage allocated in the previous tuple cycle. @@ -123,7 +122,7 @@ ExecDML(DMLState *node) isUpdate, InvalidOid); } - else /* DML_DELETE */ + else if(DML_DELETE == action) { int32 segid = GpIdentity.segindex; Datum ctid = slot_getattr(slot, plannode->ctidColIdx, &isnull); @@ -151,6 +150,26 @@ ExecDML(DMLState *node) PLANGEN_OPTIMIZER /* Plan origin */, isUpdate); } + else /* in place update */ + { + int32 segid = GpIdentity.segindex; + + Datum ctid = slot_getattr(slot, plannode->ctidColIdx, &isnull); + + ItemPointer tupleid = (ItemPointer) DatumGetPointer(ctid); + ItemPointerData tuple_ctid = *tupleid; + tupleid = &tuple_ctid; + + ExecUpdate( + tupleid, + segid, + NULL, //oldtuple + node->cleanedUpSlot, + NULL, //planSlot + &node->mt_epqstate, + node->ps.state, + true); + } return slot; } @@ -186,6 +205,8 @@ ExecInitDML(DML *node, EState *estate, int eflags) Plan *outerPlan = outerPlan(node); outerPlanState(dmlstate) = ExecInitNode(outerPlan, estate, eflags); + EvalPlanQualInit(&dmlstate->mt_epqstate, estate, outerPlan, NIL, 0); + /* * ORCA Plan does not seem to set junk attribute for "gp_segment_id", else we * could call the simple code below. @@ -229,7 +250,7 @@ ExecInitDML(DML *node, EState *estate, int eflags) dmlstate->junkfilter = ExecInitJunkFilter(node->plan.targetlist, dmlstate->ps.state->es_result_relation_info->ri_RelationDesc->rd_att->tdhasoid, dmlstate->cleanedUpSlot); - + estate->es_result_relation_info->ri_junkFilter = dmlstate->junkfilter; /* * We don't maintain typmod in the targetlist, so we should fixup the * junkfilter to use the same tuple descriptor as the result relation. @@ -279,6 +300,7 @@ ExecEndDML(DMLState *node) ExecFreeExprContext(&node->ps); ExecClearTuple(node->ps.ps_ResultTupleSlot); ExecClearTuple(node->cleanedUpSlot); + EvalPlanQualEnd(&node->mt_epqstate); ExecEndNode(outerPlanState(node)); EndPlanStateGpmonPkt(&node->ps); } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 3cba0ee49db4..7aada91fb574 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1455,7 +1455,7 @@ ExecUpdate(ItemPointer tupleid, * redo triggers, however. If there are any BEFORE triggers then * trigger.c will have done heap_lock_tuple to lock the correct tuple, * so there's no need to do them again.) - */ + */у lreplace:; if (resultRelationDesc->rd_att->constr) ExecConstraints(resultRelInfo, slot, estate); diff --git a/src/backend/gpopt/gpopt.mk b/src/backend/gpopt/gpopt.mk index 2d5bb258b26b..dfb291c4b3d6 100644 --- a/src/backend/gpopt/gpopt.mk +++ b/src/backend/gpopt/gpopt.mk @@ -1,4 +1,3 @@ -override CPPFLAGS := -std=c++98 $(CPPFLAGS) override CPPFLAGS := -I$(top_builddir)/src/backend/gporca/libgpos/include $(CPPFLAGS) override CPPFLAGS := -I$(top_builddir)/src/backend/gporca/libgpopt/include $(CPPFLAGS) override CPPFLAGS := -I$(top_builddir)/src/backend/gporca/libnaucrates/include $(CPPFLAGS) diff --git a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp index 13d594aa70fd..56f454571bad 100644 --- a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp +++ b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp @@ -4084,6 +4084,7 @@ CTranslatorDXLToPlStmt::TranslateDXLDml( DML *dml = MakeNode(DML); Plan *plan = &(dml->plan); AclMode acl_mode = ACL_NO_RIGHTS; + BOOL isSplit = phy_dml_dxlop->FSplit(); switch (phy_dml_dxlop->GetDmlOpType()) { @@ -4170,14 +4171,26 @@ CTranslatorDXLToPlStmt::TranslateDXLDml( dml_target_list = target_list_with_dropped_cols; } - // Extract column numbers of the action and ctid columns from the - // target list. - dml->actionColIdx = AddTargetEntryForColId(&dml_target_list, &child_context, - phy_dml_dxlop->ActionColId(), - true /*is_resjunk*/); + dml->ctidColIdx = AddTargetEntryForColId(&dml_target_list, &child_context, phy_dml_dxlop->GetCtIdColId(), true /*is_resjunk*/); + + // Doesn't needed for in place update + if (isSplit || CMD_UPDATE != m_cmd_type) + { + // Extract column numbers of the action and ctid columns from the + // target list. + dml->actionColIdx = AddTargetEntryForColId( + &dml_target_list, &child_context, phy_dml_dxlop->ActionColId(), + true /*is_resjunk*/); + GPOS_ASSERT(0 != dml->actionColIdx); + } + else + { + dml->actionColIdx = 0; + } + if (phy_dml_dxlop->IsOidsPreserved()) { dml->tupleoidColIdx = AddTargetEntryForColId( @@ -4189,8 +4202,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDml( dml->tupleoidColIdx = 0; } - GPOS_ASSERT(0 != dml->actionColIdx); - plan->targetlist = dml_target_list; plan->lefttree = child_plan; diff --git a/src/backend/gporca/gporca.mk b/src/backend/gporca/gporca.mk index 0bbedd33396b..d5f49e648b7f 100644 --- a/src/backend/gporca/gporca.mk +++ b/src/backend/gporca/gporca.mk @@ -5,4 +5,3 @@ override CPPFLAGS := -I$(top_builddir)/src/backend/gporca/libgpdbcost/include $( # Do not omit frame pointer. Even with RELEASE builds, it is used for # backtracing. override CPPFLAGS := -Werror -Wextra -Wpedantic -Wno-variadic-macros -fno-omit-frame-pointer $(CPPFLAGS) -override CPPFLAGS := -std=gnu++98 $(CPPFLAGS) diff --git a/src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp b/src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp index 727b6ba161a0..bd1a8adb14ad 100644 --- a/src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp +++ b/src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp @@ -3315,9 +3315,10 @@ CExpressionPreprocessor::ConvertSplitUpdateToInPlaceUpdate(CMemoryPool *mp, { CColRef *pcrInsert = (*pdrgpcrInsert)[ul]; CColRef *pcrDelete = (*pdrgpcrDelete)[ul]; + // Checking if column is either distribution or partition key. - if (pcrInsert != pcrDelete && - (pcrDelete->IsDistCol() || ppartColRefs->Find(pcrInsert) != NULL)) + if ((pcrInsert != pcrDelete && pcrDelete->IsDistCol()) || + (ppartColRefs->Find(pcrInsert) != NULL)) { split_update = true; break; diff --git a/src/backend/gporca/libgpopt/src/xforms/CXformUpdate2DML.cpp b/src/backend/gporca/libgpopt/src/xforms/CXformUpdate2DML.cpp index 522928472c01..80249bf2f82e 100644 --- a/src/backend/gporca/libgpopt/src/xforms/CXformUpdate2DML.cpp +++ b/src/backend/gporca/libgpopt/src/xforms/CXformUpdate2DML.cpp @@ -151,26 +151,6 @@ CXformUpdate2DML::Transform(CXformContext *pxfctxt, CXformResult *pxfres, pexprAssertConstraints = pexprSplit; } - // generate oid column and project operator - CExpression *pexprProject = NULL; - if (ptabdesc->IsPartitioned()) - { - // generate a partition selector - pexprProject = CXformUtils::PexprLogicalPartitionSelector( - mp, ptabdesc, pdrgpcrInsert, pexprAssertConstraints); - } - else - { - // generate a project operator - IMDId *pmdidTable = ptabdesc->MDId(); - - OID oidTable = CMDIdGPDB::CastMdid(pmdidTable)->Oid(); - CExpression *pexprOid = CUtils::PexprScalarConstOid(mp, oidTable); - - pexprProject = - CUtils::PexprAddProjection(mp, pexprAssertConstraints, pexprOid); - } - const ULONG num_cols = pdrgpcrInsert->Size(); CExpression *pexprDML = NULL; @@ -199,7 +179,7 @@ CXformUpdate2DML::Transform(CXformContext *pxfctxt, CXformResult *pxfres, CLogicalDML(mp, CLogicalDML::EdmlUpdate, ptabdesc, pdrgpcrDelete, pbsModified, pcrAction, pcrCtid, pcrSegmentId, pcrTupleOid, fSplit), - pexprProject); + pexprAssertConstraints); } else { @@ -210,7 +190,7 @@ CXformUpdate2DML::Transform(CXformContext *pxfctxt, CXformResult *pxfres, CLogicalDML(mp, CLogicalDML::EdmlUpdate, ptabdesc, pdrgpcrInsert, GPOS_NEW(mp) CBitSet(mp), pcrAction, pcrCtid, pcrSegmentId, NULL, fSplit), - pexprProject); + pexprAssertConstraints); } // TODO: - Oct 30, 2012; detect and handle AFTER triggers on update diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 0c6692e9eac9..cfb9238304c7 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2831,6 +2831,7 @@ typedef struct DMLState JunkFilter *junkfilter; /* filter that removes junk and dropped attributes */ TupleTableSlot *cleanedUpSlot; /* holds 'final' tuple which matches the target relation schema */ AttrNumber segid_attno; /* attribute number of "gp_segment_id" */ + EPQState mt_epqstate; /* for evaluating EvalPlanQual rechecks */ } DMLState; /* diff --git a/src/test/isolation2/expected/gdd/concurrent_update_optimizer.out b/src/test/isolation2/expected/gdd/concurrent_update_optimizer.out index da71357fdba3..097f3fe7f8ea 100644 --- a/src/test/isolation2/expected/gdd/concurrent_update_optimizer.out +++ b/src/test/isolation2/expected/gdd/concurrent_update_optimizer.out @@ -353,29 +353,28 @@ UPDATE 1 -- make sure planner will contain InitPlan -- NOTE: orca does not generate InitPlan. 2: explain update t_epq_subplans set b = b + 1 where a > -1.5 * (select max(a) from t_epq_subplans); - QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------------------------ - Update (cost=0.00..1324032.68 rows=1 width=1) - -> Split (cost=0.00..1324032.61 rows=1 width=22) - -> Result (cost=0.00..1324032.61 rows=1 width=22) - -> Seq Scan on t_epq_subplans (cost=0.00..1324032.61 rows=1 width=18) - Filter: ((a)::numeric > ((-1.5) * ((SubPlan 1))::numeric)) - SubPlan 1 (slice0; segments: 3) - -> Materialize (cost=0.00..431.00 rows=1 width=4) - -> Broadcast Motion 1:3 (slice2; segments: 1) (cost=0.00..431.00 rows=3 width=4) - -> Aggregate (cost=0.00..431.00 rows=1 width=4) - -> Gather Motion 3:1 (slice1; segments: 3) (cost=0.00..431.00 rows=1 width=4) - -> Aggregate (cost=0.00..431.00 rows=1 width=4) - -> Seq Scan on t_epq_subplans t_epq_subplans_1 (cost=0.00..431.00 rows=1 width=4) - Optimizer: Pivotal Optimizer (GPORCA) -(14 rows) + QUERY PLAN +----------------------------------------------------------------------------------------------------------------------------------- + Update (cost=0.00..1324032.63 rows=1 width=1) + -> Result (cost=0.00..1324032.61 rows=1 width=18) + -> Seq Scan on t_epq_subplans (cost=0.00..1324032.61 rows=1 width=18) + Filter: ((a)::numeric > ('-1.5'::numeric * ((SubPlan 1))::numeric)) + SubPlan 1 (slice0; segments: 3) + -> Materialize (cost=0.00..431.00 rows=1 width=4) + -> Broadcast Motion 1:3 (slice2; segments: 1) (cost=0.00..431.00 rows=3 width=4) + -> Aggregate (cost=0.00..431.00 rows=1 width=4) + -> Gather Motion 3:1 (slice1; segments: 3) (cost=0.00..431.00 rows=1 width=4) + -> Aggregate (cost=0.00..431.00 rows=1 width=4) + -> Seq Scan on t_epq_subplans t_epq_subplans_1 (cost=0.00..431.00 rows=1 width=4) + Optimizer: Pivotal Optimizer (GPORCA) +(12 rows) 2&: update t_epq_subplans set b = b + 1 where a > -1.5 * (select max(a) from t_epq_subplans); 1: end; END -- session 2 should throw error and not PANIC 2<: <... completed> -ERROR: tuple to be updated was already moved to another segment due to concurrent update (seg1 127.0.1.1:6003 pid=116712) +ERROR: EvalPlanQual can not handle subPlan with Motion node (seg1 172.17.0.2:7003 pid=808) 1: drop table t_epq_subplans; DROP diff --git a/src/test/isolation2/expected/modify_table_data_corrupt_optimizer.out b/src/test/isolation2/expected/modify_table_data_corrupt_optimizer.out index b313e608b524..8b2830c595ce 100644 --- a/src/test/isolation2/expected/modify_table_data_corrupt_optimizer.out +++ b/src/test/isolation2/expected/modify_table_data_corrupt_optimizer.out @@ -107,25 +107,24 @@ ABORT -- TODO: this case is for planner, it will not error out on 6X now, -- because 6x does not remove explicit motion yet. explain (costs off) update tab1 set a = 999 from tab2, tab3 where tab1.a = tab2.a and tab1.b = tab3.b; - QUERY PLAN ---------------------------------------------------------------------------------- - Update - -> Redistribute Motion 3:3 (slice2; segments: 3) - Hash Key: tab1.b - -> Split - -> Result - -> Hash Join - Hash Cond: (tab2.a = tab1.a) - -> Seq Scan on tab2 - -> Hash - -> Broadcast Motion 3:3 (slice1; segments: 3) - -> Hash Join - Hash Cond: (tab3.b = tab1.b) - -> Seq Scan on tab3 - -> Hash - -> Seq Scan on tab1 - Optimizer: Pivotal Optimizer (GPORCA) -(16 rows) + QUERY PLAN +--------------------------------------------------------------------------- + Update + -> Result + -> Redistribute Motion 3:3 (slice2; segments: 3) + Hash Key: tab1.b + -> Hash Join + Hash Cond: (tab2.a = tab1.a) + -> Seq Scan on tab2 + -> Hash + -> Broadcast Motion 3:3 (slice1; segments: 3) + -> Hash Join + Hash Cond: (tab3.b = tab1.b) + -> Seq Scan on tab3 + -> Hash + -> Seq Scan on tab1 + Optimizer: Pivotal Optimizer (GPORCA) +(15 rows) begin; BEGIN update tab1 set a = 999 from tab2, tab3 where tab1.a = tab2.a and tab1.b = tab3.b; @@ -186,7 +185,7 @@ explain (costs off) update tab1 set a = 999 from tab2, tab3 where tab1.a = tab2. begin; BEGIN update tab1 set a = 999 from tab2, tab3 where tab1.a = tab2.a and tab1.b = tab3.a; -ERROR: distribution key of the tuple (0, 1) doesn't belong to current segment (actually from seg0) (nodeModifyTable.c:602) (seg1 127.0.1.1:6003 pid=78344) (nodeModifyTable.c:602) +UPDATE 1 abort; ABORT diff --git a/src/test/regress/expected/gporca_optimizer.out b/src/test/regress/expected/gporca_optimizer.out index e71cfa6996d6..0899b79cf569 100644 --- a/src/test/regress/expected/gporca_optimizer.out +++ b/src/test/regress/expected/gporca_optimizer.out @@ -13733,7 +13733,7 @@ and first_id in (select first_id from mat_w); Output: share0_ref2.first_id Optimizer: Pivotal Optimizer (GPORCA) Settings: optimizer=on -(30 rows) +(31 rows) with mat_w as ( select first_id diff --git a/src/test/regress/expected/qp_dml_joins_optimizer.out b/src/test/regress/expected/qp_dml_joins_optimizer.out index 5e7a78999d92..06c448499c5d 100644 --- a/src/test/regress/expected/qp_dml_joins_optimizer.out +++ b/src/test/regress/expected/qp_dml_joins_optimizer.out @@ -4080,7 +4080,7 @@ UPDATE dml_heap_v SET a = dml_heap_u.a FROM dml_heap_u WHERE dml_heap_u.b = dml_ SELECT SUM(a) FROM dml_heap_v; sum ----- - 63 + 64 (1 row) --Update with joins on multiple table