Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADBDEV-3951: Backport of "Implemented InPlaceUpdate to be used for updates made on non-distribution columns" #609

Open
wants to merge 2 commits into
base: adb-6.x-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 41 additions & 20 deletions src/backend/executor/nodeDML.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,17 @@ ExecDML(DMLState *node)
}

bool isnull = false;
int action = DatumGetUInt32(slot_getattr(slot, plannode->actionColIdx, &isnull));
Assert(!isnull);

bool isUpdate = false;
if (node->ps.state->es_plannedstmt->commandType == CMD_UPDATE)
int action = -1;
bool isUpdate = node->ps.state->es_plannedstmt->commandType == CMD_UPDATE;
// if it's not in place update
if (AttributeNumberIsValid(plannode->actionColIdx))
{
isUpdate = true;
action = DatumGetUInt32(
slot_getattr(slot, plannode->actionColIdx, &isnull));
Assert(!isnull);
Assert(action == DML_INSERT || action == DML_DELETE);
}

Assert(action == DML_INSERT || action == DML_DELETE);


/*
* Reset per-tuple memory context to free any expression evaluation
* storage allocated in the previous tuple cycle.
Expand Down Expand Up @@ -116,6 +115,13 @@ ExecDML(DMLState *node)
* es_result_relations will contain the only relation.
*/
node->ps.state->es_result_relation_info = relInfo;

/*
* Perform partition selection for in place update
*/
if (isUpdate && !AttributeNumberIsValid(plannode->actionColIdx))
Copy link
Member

@bimboterminator1 bimboterminator1 Jan 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to perform partition selection in case of updating leaf partition. Do we?

node->ps.state->es_result_relation_info =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the tests check this code branch?

slot_get_partition(node->cleanedUpSlot, node->ps.state);
}
/* GPDB_91_MERGE_FIXME:
* This kind of node is used by ORCA only. If in the future ORCA still uses
Expand Down Expand Up @@ -155,7 +161,7 @@ ExecDML(DMLState *node)
isUpdate,
InvalidOid);
}
else /* DML_DELETE */
else
{
int32 segid = GpIdentity.segindex;
Datum ctid = slot_getattr(slot, plannode->ctidColIdx, &isnull);
Expand All @@ -172,16 +178,31 @@ ExecDML(DMLState *node)
Assert(!isnull);
}

/* Correct tuple count by ignoring deletes when splitting tuples. */
ExecDelete(tupleid,
segid,
NULL, /* GPDB_91_MERGE_FIXME: oldTuple? */
node->cleanedUpSlot,
NULL /* DestReceiver */,
node->ps.state,
!isUpdate, /* GPDB_91_MERGE_FIXME: where to get canSetTag? */
PLANGEN_OPTIMIZER /* Plan origin */,
isUpdate);
if (DML_DELETE == action)
{
/* Correct tuple count by ignoring deletes when splitting tuples. */
RekGRpth marked this conversation as resolved.
Show resolved Hide resolved
ExecDelete(tupleid,
segid,
NULL, /* GPDB_91_MERGE_FIXME: oldTuple? */
node->cleanedUpSlot,
NULL /* DestReceiver */,
node->ps.state,
!isUpdate, /* GPDB_91_MERGE_FIXME: where to get canSetTag? */
PLANGEN_OPTIMIZER /* Plan origin */,
isUpdate);
}
else
{
ExecUpdate(tupleid,
segid,
NULL, //oldtuple
node->cleanedUpSlot,
NULL, //planSlot
NULL /* DestReceiver */,
node->ps.state,
true,
PLANGEN_OPTIMIZER);
}
}

return slot;
Expand Down
20 changes: 18 additions & 2 deletions src/backend/executor/nodeModifyTable.c
Original file line number Diff line number Diff line change
Expand Up @@ -1301,7 +1301,8 @@ ExecUpdate(ItemPointer tupleid,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate,
bool canSetTag)
bool canSetTag,
PlanGenerator planGen)
{
ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
Expand Down Expand Up @@ -1370,8 +1371,12 @@ ExecUpdate(ItemPointer tupleid,

/* see if this update would move the tuple to a different partition */
if (estate->es_result_partitions)
{
checkPartitionUpdate(estate, slot, resultRelInfo);

if (planGen == PLANGEN_OPTIMIZER)
slot = reconstructMatchingTupleSlot(slot, resultRelInfo);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the tests check this code branch?

}
/* BEFORE ROW UPDATE Triggers */
if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->trig_update_before_row)
Expand Down Expand Up @@ -1571,6 +1576,17 @@ lreplace:;
{
TupleTableSlot *epqslot;

/*
* TODO: DML node doesn't initialize `epqstate` parameter so
* we exclude EPQ routine for this type of modification and
* act as in RR and upper isolation levels.
*/
if (!epqstate)
ereport(ERROR,
RekGRpth marked this conversation as resolved.
Show resolved Hide resolved
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
errmsg("could not serialize access due to concurrent update"),
errhint("Use PostgreSQL Planner instead of Optimizer for this query via optimizer=off GUC setting")));

epqslot = EvalPlanQual(estate,
epqstate,
resultRelationDesc,
Expand Down Expand Up @@ -1942,7 +1958,7 @@ ExecModifyTable(ModifyTableState *node)
slot = ExecUpdate(tupleid, segid,
oldtuple, slot, planSlot,
&node->mt_epqstate, estate,
node->canSetTag);
node->canSetTag, PLANGEN_PLANNER);
break;
}

Expand Down
23 changes: 16 additions & 7 deletions src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4100,6 +4100,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())
{
Expand Down Expand Up @@ -4186,11 +4187,21 @@ 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*/);
// 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;
}

dml->ctidColIdx = AddTargetEntryForColId(&dml_target_list, &child_context,
phy_dml_dxlop->GetCtIdColId(),
true /*is_resjunk*/);
Expand All @@ -4205,8 +4216,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDml(
dml->tupleoidColIdx = 0;
}

GPOS_ASSERT(0 != dml->actionColIdx);

plan->targetlist = dml_target_list;

plan->lefttree = child_plan;
Expand Down
81 changes: 25 additions & 56 deletions src/backend/gporca/data/dxl/minidump/SelfUpdate.mdp
Original file line number Diff line number Diff line change
Expand Up @@ -216,17 +216,17 @@ update t1 set b = c;
</dxl:LogicalUpdate>
</dxl:Query>
<dxl:Plan Id="0" SpaceSize="1">
<dxl:DMLUpdate Columns="0,1,2" ActionCol="10" CtidCol="3" SegmentIdCol="9" PreserveOids="false">
<dxl:DMLUpdate Columns="0,2,2" ActionCol="10" CtidCol="3" SegmentIdCol="9" IsSplitUpdate="false" PreserveOids="false">
<dxl:Properties>
<dxl:Cost StartupCost="0" TotalCost="431.067764" Rows="1.000000" Width="1"/>
<dxl:Cost StartupCost="0" TotalCost="431.023462" Rows="1.000000" Width="1"/>
</dxl:Properties>
<dxl:DirectDispatchInfo/>
<dxl:ProjList>
<dxl:ProjElem ColId="0" Alias="a">
<dxl:Ident ColId="0" ColName="a" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="1" Alias="b">
<dxl:Ident ColId="1" ColName="b" TypeMdid="0.23.1.0"/>
<dxl:ProjElem ColId="2" Alias="c">
<dxl:Ident ColId="2" ColName="c" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="2" Alias="c">
<dxl:Ident ColId="2" ColName="c" TypeMdid="0.23.1.0"/>
Expand All @@ -248,14 +248,14 @@ update t1 set b = c;
</dxl:TableDescriptor>
<dxl:Assert ErrorCode="23502">
<dxl:Properties>
<dxl:Cost StartupCost="0" TotalCost="431.000056" Rows="2.000000" Width="26"/>
<dxl:Cost StartupCost="0" TotalCost="431.000025" Rows="1.000000" Width="18"/>
</dxl:Properties>
<dxl:ProjList>
<dxl:ProjElem ColId="0" Alias="a">
<dxl:Ident ColId="0" ColName="a" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="1" Alias="b">
<dxl:Ident ColId="1" ColName="b" TypeMdid="0.23.1.0"/>
<dxl:ProjElem ColId="2" Alias="c">
<dxl:Ident ColId="2" ColName="c" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="2" Alias="c">
<dxl:Ident ColId="2" ColName="c" TypeMdid="0.23.1.0"/>
Expand All @@ -266,9 +266,6 @@ update t1 set b = c;
<dxl:ProjElem ColId="9" Alias="gp_segment_id">
<dxl:Ident ColId="9" ColName="gp_segment_id" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="10" Alias="ColRef_0010">
<dxl:Ident ColId="10" ColName="ColRef_0010" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
</dxl:ProjList>
<dxl:AssertConstraintList>
<dxl:AssertConstraint ErrorMessage="Not null constraint for column b of table t1 was violated">
Expand All @@ -279,17 +276,14 @@ update t1 set b = c;
</dxl:Not>
</dxl:AssertConstraint>
</dxl:AssertConstraintList>
<dxl:Split DeleteColumns="0,1,2" InsertColumns="0,2,2" ActionCol="10" CtidCol="3" SegmentIdCol="9" PreserveOids="false">
<dxl:TableScan>
<dxl:Properties>
<dxl:Cost StartupCost="0" TotalCost="431.000039" Rows="2.000000" Width="26"/>
<dxl:Cost StartupCost="0" TotalCost="431.000008" Rows="1.000000" Width="18"/>
</dxl:Properties>
<dxl:ProjList>
<dxl:ProjElem ColId="0" Alias="a">
<dxl:Ident ColId="0" ColName="a" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="1" Alias="b">
<dxl:Ident ColId="1" ColName="b" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="2" Alias="c">
<dxl:Ident ColId="2" ColName="c" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
Expand All @@ -299,48 +293,23 @@ update t1 set b = c;
<dxl:ProjElem ColId="9" Alias="gp_segment_id">
<dxl:Ident ColId="9" ColName="gp_segment_id" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="10" Alias="ColRef_0010">
<dxl:DMLAction/>
</dxl:ProjElem>
</dxl:ProjList>
<dxl:TableScan>
<dxl:Properties>
<dxl:Cost StartupCost="0" TotalCost="431.000008" Rows="1.000000" Width="22"/>
</dxl:Properties>
<dxl:ProjList>
<dxl:ProjElem ColId="0" Alias="a">
<dxl:Ident ColId="0" ColName="a" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="1" Alias="b">
<dxl:Ident ColId="1" ColName="b" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="2" Alias="c">
<dxl:Ident ColId="2" ColName="c" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="3" Alias="ctid">
<dxl:Ident ColId="3" ColName="ctid" TypeMdid="0.27.1.0"/>
</dxl:ProjElem>
<dxl:ProjElem ColId="9" Alias="gp_segment_id">
<dxl:Ident ColId="9" ColName="gp_segment_id" TypeMdid="0.23.1.0"/>
</dxl:ProjElem>
</dxl:ProjList>
<dxl:Filter/>
<dxl:TableDescriptor Mdid="6.47297780.1.1" TableName="t1">
<dxl:Columns>
<dxl:Column ColId="0" Attno="1" ColName="a" TypeMdid="0.23.1.0"/>
<dxl:Column ColId="1" Attno="2" ColName="b" TypeMdid="0.23.1.0"/>
<dxl:Column ColId="2" Attno="3" ColName="c" TypeMdid="0.23.1.0"/>
<dxl:Column ColId="3" Attno="-1" ColName="ctid" TypeMdid="0.27.1.0"/>
<dxl:Column ColId="4" Attno="-3" ColName="xmin" TypeMdid="0.28.1.0"/>
<dxl:Column ColId="5" Attno="-4" ColName="cmin" TypeMdid="0.29.1.0"/>
<dxl:Column ColId="6" Attno="-5" ColName="xmax" TypeMdid="0.28.1.0"/>
<dxl:Column ColId="7" Attno="-6" ColName="cmax" TypeMdid="0.29.1.0"/>
<dxl:Column ColId="8" Attno="-7" ColName="tableoid" TypeMdid="0.26.1.0"/>
<dxl:Column ColId="9" Attno="-8" ColName="gp_segment_id" TypeMdid="0.23.1.0"/>
</dxl:Columns>
</dxl:TableDescriptor>
</dxl:TableScan>
</dxl:Split>
<dxl:Filter/>
<dxl:TableDescriptor Mdid="6.47297780.1.1" TableName="t1">
<dxl:Columns>
<dxl:Column ColId="0" Attno="1" ColName="a" TypeMdid="0.23.1.0"/>
<dxl:Column ColId="1" Attno="2" ColName="b" TypeMdid="0.23.1.0"/>
<dxl:Column ColId="2" Attno="3" ColName="c" TypeMdid="0.23.1.0"/>
<dxl:Column ColId="3" Attno="-1" ColName="ctid" TypeMdid="0.27.1.0"/>
<dxl:Column ColId="4" Attno="-3" ColName="xmin" TypeMdid="0.28.1.0"/>
<dxl:Column ColId="5" Attno="-4" ColName="cmin" TypeMdid="0.29.1.0"/>
<dxl:Column ColId="6" Attno="-5" ColName="xmax" TypeMdid="0.28.1.0"/>
<dxl:Column ColId="7" Attno="-6" ColName="cmax" TypeMdid="0.29.1.0"/>
<dxl:Column ColId="8" Attno="-7" ColName="tableoid" TypeMdid="0.26.1.0"/>
<dxl:Column ColId="9" Attno="-8" ColName="gp_segment_id" TypeMdid="0.23.1.0"/>
</dxl:Columns>
</dxl:TableDescriptor>
</dxl:TableScan>
</dxl:Assert>
</dxl:DMLUpdate>
</dxl:Plan>
Expand Down
Loading
Loading