From: Tom Lane <tgl@sss.pgh.pa.us>
To: hisahiro@freemind.co.jp
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17823: Generated columns not always updated correctly
Date: Mon, 06 Mar 2023 11:35:29 -0500
Message-ID: <3402993.1678120529@sss.pgh.pa.us> (raw)
In-Reply-To: <17823-b64909cf7d63de84@postgresql.org>
References: <17823-b64909cf7d63de84@postgresql.org>
PG Bug reporting form <noreply@postgresql.org> writes:
> I found that the generated columns are sometimes not updated.
Yeah. Looking into nodeModifyTable.c, we miss re-doing
ExecComputeStoredGenerated when looping back after an EPQ update
(which is what this case is). I see that we also fail to redo that
after a cross-partition move, which is a bug since 8bf6ec3ba.
The attached seems to be enough to fix it, but I want to also devise
an isolation test for these cases ...
regards, tom lane
Attachments:
[text/x-diff] fix-bug-17823-wip.patch (2.8K, ../3402993.1678120529@sss.pgh.pa.us/2-fix-bug-17823-wip.patch)
download | inline diff:diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.cindex 6f0543af83..6d9c0b0806 100644--- a/src/backend/executor/nodeModifyTable.c+++ b/src/backend/executor/nodeModifyTable.c@@ -1910,9 +1910,10 @@ ExecUpdatePrologue(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
}
/*
- * ExecUpdatePrepareSlot -- subroutine for ExecUpdate+ * ExecUpdatePrepareSlot -- subroutine for ExecUpdateAct
*
* Apply the final modifications to the tuple slot before the update.
+ * (This is split out because we also need it in the foreign-table code path.)
*/
static void
ExecUpdatePrepareSlot(ResultRelInfo *resultRelInfo,
@@ -1962,13 +1963,14 @@ ExecUpdateAct(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
updateCxt->crossPartUpdate = false;
/*
- * If we generate a new candidate tuple after EvalPlanQual testing, we- * must loop back here and recheck any RLS policies and constraints. (We- * don't need to redo triggers, however. If there are any BEFORE triggers- * then trigger.c will have done table_tuple_lock to lock the correct- * tuple, so there's no need to do them again.)+ * If we move the tuple to a new partition, we loop back here to recompute+ * GENERATED values (which are allowed to be different across partitions)+ * and recheck any RLS policies and constraints. We do not fire any+ * BEFORE triggers of the new partition, however.
*/
lreplace:
+ /* Fill in GENERATEd columns */+ ExecUpdatePrepareSlot(resultRelInfo, slot, estate);
/* ensure slot is independent, consider e.g. EPQ */
ExecMaterializeSlot(slot);
@@ -2268,6 +2270,7 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
}
else if (resultRelInfo->ri_FdwRoutine)
{
+ /* Fill in GENERATEd columns */
ExecUpdatePrepareSlot(resultRelInfo, slot, estate);
/*
@@ -2290,9 +2293,13 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
}
else
{
- /* Fill in the slot appropriately */- ExecUpdatePrepareSlot(resultRelInfo, slot, estate);-+ /*+ * If we generate a new candidate tuple after EvalPlanQual testing, we+ * must loop back here to try again. (We don't need to redo triggers,+ * however. If there are any BEFORE triggers then trigger.c will have+ * done table_tuple_lock to lock the correct tuple, so there's no need+ * to do them again.)+ */
redo_act:
result = ExecUpdateAct(context, resultRelInfo, tupleid, oldtuple, slot,
canSetTag, &updateCxt);
@@ -2876,7 +2883,6 @@ lmerge_matched:
result = TM_Ok;
break;
}
- ExecUpdatePrepareSlot(resultRelInfo, newslot, context->estate);
result = ExecUpdateAct(context, resultRelInfo, tupleid, NULL,
newslot, false, &updateCxt);
if (result == TM_Ok && updateCxt.updated)
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-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, hisahiro@freemind.co.jp, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17823: Generated columns not always updated correctly
In-Reply-To: <3402993.1678120529@sss.pgh.pa.us>
* 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