Re: BUG #17823: Generated columns not always updated correctly - Mailing list pgsql-bugs

From Tom Lane
Subject Re: BUG #17823: Generated columns not always updated correctly
Date
Msg-id 3606820.1678137490@sss.pgh.pa.us
Whole thread Raw
In response to Re: BUG #17823: Generated columns not always updated correctly  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-bugs
I wrote:
> 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 ...

Building a test case for cross-partition updates showed that there's
a second problem: when we convert an UPDATE into an INSERT on another
partition, we really need to compute all the other partition's
GENERATED columns (as a real INSERT would); but we might only compute
some of them, if we'd already initialized the target partition's
ri_GeneratedExprs data as for an UPDATE.

AFAICS the only true fix for this is to keep separate ri_GeneratedExprs
data for INSERT and UPDATE cases.  Most of the time we'd only compute
one set for any given target partition; but a given partition could
receive both local UPDATEs and cross-partition INSERTs in the same
UPDATE command, so it can happen that we need both.

Hence the attached.  One improvement we can make is to drop the
early call of ExecInitStoredGenerated in ExecInitModifyTable, and
calculate this stuff only upon-demand.  The claim that we have to
do it to pre-fill ri_extraUpdatedCols is wrong given that
ExecGetExtraUpdatedCols now knows to call ExecInitStoredGenerated.

I'm not sure how much of this needs to be back-patched.  I think that
before 8bf6ec3ba, it might not matter if a cross-partition INSERT misses
doing some of the target partition's GENERATED expressions, since they
should match those of the source partition which we'd have already
computed correctly in the to-be-inserted tuple.

            regards, tom lane

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 7f5968db87..b32f419176 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1232,7 +1232,8 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
     resultRelInfo->ri_FdwState = NULL;
     resultRelInfo->ri_usesFdwDirectModify = false;
     resultRelInfo->ri_ConstraintExprs = NULL;
-    resultRelInfo->ri_GeneratedExprs = NULL;
+    resultRelInfo->ri_GeneratedExprsI = NULL;
+    resultRelInfo->ri_GeneratedExprsU = NULL;
     resultRelInfo->ri_projectReturning = NULL;
     resultRelInfo->ri_onConflictArbiterIndexes = NIL;
     resultRelInfo->ri_onConflict = NULL;
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 6eac5f354c..012dbb6965 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -1339,8 +1339,8 @@ ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate)
 Bitmapset *
 ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate)
 {
-    /* In some code paths we can reach here before initializing the info */
-    if (relinfo->ri_GeneratedExprs == NULL)
+    /* Compute the info if we didn't already */
+    if (relinfo->ri_GeneratedExprsU == NULL)
         ExecInitStoredGenerated(relinfo, estate, CMD_UPDATE);
     return relinfo->ri_extraUpdatedCols;
 }
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 6f0543af83..49f4d57d5f 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -356,9 +356,14 @@ ExecCheckTIDVisible(EState *estate,
 /*
  * Initialize to compute stored generated columns for a tuple
  *
- * This fills the resultRelInfo's ri_GeneratedExprs and ri_extraUpdatedCols
- * fields.  (Currently, ri_extraUpdatedCols is consulted only in UPDATE,
- * but we might as well fill it for INSERT too.)
+ * This fills the resultRelInfo's ri_GeneratedExprsI/ri_NumGeneratedNeededI
+ * or ri_GeneratedExprsU/ri_NumGeneratedNeededU fields, depending on cmdtype.
+ * If cmdType == CMD_UPDATE, the ri_extraUpdatedCols field is filled too.
+ *
+ * Note: usually, a given query would need only one of ri_GeneratedExprsI and
+ * ri_GeneratedExprsU per result rel; but cross-partition UPDATEs can need
+ * both, since a partition might be the target of both UPDATE and INSERT
+ * actions.
  */
 void
 ExecInitStoredGenerated(ResultRelInfo *resultRelInfo,
@@ -368,12 +373,11 @@ ExecInitStoredGenerated(ResultRelInfo *resultRelInfo,
     Relation    rel = resultRelInfo->ri_RelationDesc;
     TupleDesc    tupdesc = RelationGetDescr(rel);
     int            natts = tupdesc->natts;
+    ExprState **ri_GeneratedExprs;
+    int            ri_NumGeneratedNeeded;
     Bitmapset  *updatedCols;
     MemoryContext oldContext;

-    /* Don't call twice */
-    Assert(resultRelInfo->ri_GeneratedExprs == NULL);
-
     /* Nothing to do if no generated columns */
     if (!(tupdesc->constr && tupdesc->constr->has_generated_stored))
         return;
@@ -396,9 +400,8 @@ ExecInitStoredGenerated(ResultRelInfo *resultRelInfo,
      */
     oldContext = MemoryContextSwitchTo(estate->es_query_cxt);

-    resultRelInfo->ri_GeneratedExprs =
-        (ExprState **) palloc0(natts * sizeof(ExprState *));
-    resultRelInfo->ri_NumGeneratedNeeded = 0;
+    ri_GeneratedExprs = (ExprState **) palloc0(natts * sizeof(ExprState *));
+    ri_NumGeneratedNeeded = 0;

     for (int i = 0; i < natts; i++)
     {
@@ -427,16 +430,35 @@ ExecInitStoredGenerated(ResultRelInfo *resultRelInfo,
             }

             /* No luck, so prepare the expression for execution */
-            resultRelInfo->ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
-            resultRelInfo->ri_NumGeneratedNeeded++;
-
-            /* And mark this column in resultRelInfo->ri_extraUpdatedCols */
-            resultRelInfo->ri_extraUpdatedCols =
-                bms_add_member(resultRelInfo->ri_extraUpdatedCols,
-                               i + 1 - FirstLowInvalidHeapAttributeNumber);
+            ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
+            ri_NumGeneratedNeeded++;
+
+            /* If UPDATE, mark column in resultRelInfo->ri_extraUpdatedCols */
+            if (cmdtype == CMD_UPDATE)
+                resultRelInfo->ri_extraUpdatedCols =
+                    bms_add_member(resultRelInfo->ri_extraUpdatedCols,
+                                   i + 1 - FirstLowInvalidHeapAttributeNumber);
         }
     }

+    /* Save in appropriate set of fields */
+    if (cmdtype == CMD_UPDATE)
+    {
+        /* Don't call twice */
+        Assert(resultRelInfo->ri_GeneratedExprsU == NULL);
+
+        resultRelInfo->ri_GeneratedExprsU = ri_GeneratedExprs;
+        resultRelInfo->ri_NumGeneratedNeededU = ri_NumGeneratedNeeded;
+    }
+    else
+    {
+        /* Don't call twice */
+        Assert(resultRelInfo->ri_GeneratedExprsI == NULL);
+
+        resultRelInfo->ri_GeneratedExprsI = ri_GeneratedExprs;
+        resultRelInfo->ri_NumGeneratedNeededI = ri_NumGeneratedNeeded;
+    }
+
     MemoryContextSwitchTo(oldContext);
 }

@@ -452,6 +474,7 @@ ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
     TupleDesc    tupdesc = RelationGetDescr(rel);
     int            natts = tupdesc->natts;
     ExprContext *econtext = GetPerTupleExprContext(estate);
+    ExprState **ri_GeneratedExprs;
     MemoryContext oldContext;
     Datum       *values;
     bool       *nulls;
@@ -460,20 +483,25 @@ ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
     Assert(tupdesc->constr && tupdesc->constr->has_generated_stored);

     /*
-     * For relations named directly in the query, ExecInitStoredGenerated
-     * should have been called already; but this might not have happened yet
-     * for a partition child rel.  Also, it's convenient for outside callers
-     * to not have to call ExecInitStoredGenerated explicitly.
-     */
-    if (resultRelInfo->ri_GeneratedExprs == NULL)
-        ExecInitStoredGenerated(resultRelInfo, estate, cmdtype);
-
-    /*
-     * If no generated columns have been affected by this change, then skip
-     * the rest.
+     * Initialize the expressions if we didn't already, and check whether we
+     * can exit early because nothing needs to be computed.
      */
-    if (resultRelInfo->ri_NumGeneratedNeeded == 0)
-        return;
+    if (cmdtype == CMD_UPDATE)
+    {
+        if (resultRelInfo->ri_GeneratedExprsU == NULL)
+            ExecInitStoredGenerated(resultRelInfo, estate, cmdtype);
+        if (resultRelInfo->ri_NumGeneratedNeededU == 0)
+            return;
+        ri_GeneratedExprs = resultRelInfo->ri_GeneratedExprsU;
+    }
+    else
+    {
+        if (resultRelInfo->ri_GeneratedExprsI == NULL)
+            ExecInitStoredGenerated(resultRelInfo, estate, cmdtype);
+        /* Early exit is impossible given the prior Assert */
+        Assert(resultRelInfo->ri_NumGeneratedNeededI > 0);
+        ri_GeneratedExprs = resultRelInfo->ri_GeneratedExprsI;
+    }

     oldContext = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));

@@ -487,7 +515,7 @@ ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
     {
         Form_pg_attribute attr = TupleDescAttr(tupdesc, i);

-        if (resultRelInfo->ri_GeneratedExprs[i])
+        if (ri_GeneratedExprs[i])
         {
             Datum        val;
             bool        isnull;
@@ -496,7 +524,7 @@ ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,

             econtext->ecxt_scantuple = slot;

-            val = ExecEvalExpr(resultRelInfo->ri_GeneratedExprs[i], econtext, &isnull);
+            val = ExecEvalExpr(ri_GeneratedExprs[i], econtext, &isnull);

             /*
              * We must make a copy of val as we have no guarantees about where
@@ -1910,9 +1938,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 +1991,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 +2298,7 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
     }
     else if (resultRelInfo->ri_FdwRoutine)
     {
+        /* Fill in GENERATEd columns */
         ExecUpdatePrepareSlot(resultRelInfo, slot, estate);

         /*
@@ -2290,9 +2321,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 +2911,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)
@@ -4135,15 +4169,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
                     elog(ERROR, "could not find junk wholerow column");
             }
         }
-
-        /*
-         * For INSERT/UPDATE/MERGE, prepare to evaluate any generated columns.
-         * We must do this now, even if we never insert or update any rows,
-         * because we have to fill resultRelInfo->ri_extraUpdatedCols for
-         * possible use by the trigger machinery.
-         */
-        if (operation == CMD_INSERT || operation == CMD_UPDATE || operation == CMD_MERGE)
-            ExecInitStoredGenerated(resultRelInfo, estate, operation);
     }

     /*
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index e7eb1e666f..bc67cb9ed8 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -462,7 +462,7 @@ typedef struct ResultRelInfo
      */
     AttrNumber    ri_RowIdAttNo;

-    /* For INSERT/UPDATE, attnums of generated columns to be computed */
+    /* For UPDATE, attnums of generated columns to be computed */
     Bitmapset  *ri_extraUpdatedCols;

     /* Projection to generate new tuple in an INSERT/UPDATE */
@@ -516,11 +516,13 @@ typedef struct ResultRelInfo
     /* array of constraint-checking expr states */
     ExprState **ri_ConstraintExprs;

-    /* array of stored generated columns expr states */
-    ExprState **ri_GeneratedExprs;
+    /* arrays of stored generated columns expr states, for INSERT and UPDATE */
+    ExprState **ri_GeneratedExprsI;
+    ExprState **ri_GeneratedExprsU;

     /* number of stored generated columns we need to compute */
-    int            ri_NumGeneratedNeeded;
+    int            ri_NumGeneratedNeededI;
+    int            ri_NumGeneratedNeededU;

     /* list of RETURNING expressions */
     List       *ri_returningList;
diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out
index d9063500d3..6af262ec5d 100644
--- a/src/test/isolation/expected/eval-plan-qual.out
+++ b/src/test/isolation/expected/eval-plan-qual.out
@@ -17,10 +17,10 @@ balance

 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |    850
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |    850|    1700
+savings  |    600|    1200
 (2 rows)


@@ -40,10 +40,10 @@ balance

 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   1100
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |   1100|    2200
+savings  |    600|    1200
 (2 rows)


@@ -64,10 +64,10 @@ balance

 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   1050
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |   1050|    2100
+savings  |    600|    1200
 (2 rows)


@@ -88,10 +88,10 @@ balance

 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   1600
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |   1600|    3200
+savings  |    600|    1200
 (2 rows)


@@ -117,9 +117,9 @@ balance

 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+savings  |    600|    1200
 (1 row)


@@ -140,9 +140,9 @@ balance

 step c1: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+savings  |    600|    1200
 (1 row)


@@ -168,10 +168,10 @@ balance

 step c1: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   1500
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |   1500|    3000
+savings  |    600|    1200
 (2 rows)


@@ -192,9 +192,9 @@ balance

 step c1: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+savings  |    600|    1200
 (1 row)


@@ -221,10 +221,10 @@ balance

 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   1050
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |   1050|    2100
+savings  |    600|    1200
 (2 rows)


@@ -245,9 +245,9 @@ balance

 step c1: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+savings  |    600|    1200
 (1 row)


@@ -274,9 +274,9 @@ balance

 step c1: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+savings  |    600|    1200
 (1 row)


@@ -298,9 +298,9 @@ balance

 step c1: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+savings  |    600|    1200
 (1 row)


@@ -320,9 +320,9 @@ balance

 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+savings  |    600|    1200
 (1 row)


@@ -343,10 +343,10 @@ balance

 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   1050
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |   1050|    2100
+savings  |    600|    1200
 (2 rows)


@@ -371,10 +371,10 @@ step wnested2:
 step c1: COMMIT;
 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   -600
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |   -600|   -1200
+savings  |    600|    1200
 (2 rows)


@@ -420,10 +420,10 @@ s2: NOTICE:  upid: text savings = text checking: f
 step wnested2: <... completed>
 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   -800
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |   -800|   -1600
+savings  |    600|    1200
 (2 rows)


@@ -471,10 +471,10 @@ s2: NOTICE:  upid: text savings = text checking: f
 step wnested2: <... completed>
 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |    200
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |    200|     400
+savings  |    600|    1200
 (2 rows)


@@ -527,10 +527,10 @@ s2: NOTICE:  upid: text savings = text checking: f
 step wnested2: <... completed>
 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |    200
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |    200|     400
+savings  |    600|    1200
 (2 rows)


@@ -577,10 +577,10 @@ s2: NOTICE:  upid: text savings = text checking: f
 step wnested2: <... completed>
 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |    400
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |    400|     800
+savings  |    600|    1200
 (2 rows)


@@ -614,10 +614,10 @@ s2: NOTICE:  upid: text savings = text checking: f
 step wnested2: <... completed>
 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-cds      |    400
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+cds      |    400|     800
+savings  |    600|    1200
 (2 rows)


@@ -652,10 +652,10 @@ s2: NOTICE:  upid: text savings = text checking: f
 step wnested2: <... completed>
 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |    400
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |    400|     800
+savings  |    600|    1200
 (2 rows)


@@ -669,17 +669,17 @@ balance
 step updwcte: WITH doup AS (UPDATE accounts SET balance = balance + 1100 WHERE accountid = 'checking' RETURNING *)
UPDATEaccounts a SET balance = doup.balance + 100 FROM doup RETURNING *; <waiting ...> 
 step c1: COMMIT;
 step updwcte: <... completed>
-accountid|balance|accountid|balance
----------+-------+---------+-------
-savings  |   1600|checking |   1500
+accountid|balance|balance2|accountid|balance|balance2
+---------+-------+--------+---------+-------+--------
+savings  |   1600|    3200|checking |   1500|    3000
 (1 row)

 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   1500
-savings  |   1600
+accountid|balance|balance2
+---------+-------+--------
+checking |   1500|    3000
+savings  |   1600|    3200
 (2 rows)


@@ -696,10 +696,10 @@ step updwctefail: <... completed>
 ERROR:  tuple to be updated was already modified by an operation triggered by the current command
 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |    400
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |    400|     800
+savings  |    600|    1200
 (2 rows)


@@ -713,16 +713,16 @@ balance
 step delwcte: WITH doup AS (UPDATE accounts SET balance = balance + 1100 WHERE accountid = 'checking' RETURNING *)
DELETEFROM accounts a USING doup RETURNING *; <waiting ...> 
 step c1: COMMIT;
 step delwcte: <... completed>
-accountid|balance|accountid|balance
----------+-------+---------+-------
-savings  |    600|checking |   1500
+accountid|balance|balance2|accountid|balance|balance2
+---------+-------+--------+---------+-------+--------
+savings  |    600|    1200|checking |   1500|    3000
 (1 row)

 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   1500
+accountid|balance|balance2
+---------+-------+--------
+checking |   1500|    3000
 (1 row)


@@ -739,10 +739,10 @@ step delwctefail: <... completed>
 ERROR:  tuple to be deleted was already modified by an operation triggered by the current command
 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |    400
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |    400|     800
+savings  |    600|    1200
 (2 rows)


@@ -767,10 +767,10 @@ step c1: COMMIT;
 step upsert2: <... completed>
 step c2: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |    600
-savings  |   2334
+accountid|balance|balance2
+---------+-------+--------
+checking |    600|    1200
+savings  |   2334|    4668
 (2 rows)


@@ -856,18 +856,18 @@ step partiallock:
  <waiting ...>
 step c2: COMMIT;
 step partiallock: <... completed>
-accountid|balance|accountid|balance
----------+-------+---------+-------
-checking |   1050|checking |    600
-savings  |    600|savings  |    600
+accountid|balance|balance2|accountid|balance|balance2
+---------+-------+--------+---------+-------+--------
+checking |   1050|    2100|checking |    600|    1200
+savings  |    600|    1200|savings  |    600|    1200
 (2 rows)

 step c1: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   1050
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |   1050|    2100
+savings  |    600|    1200
 (2 rows)


@@ -887,18 +887,18 @@ step lockwithvalues:
  <waiting ...>
 step c2: COMMIT;
 step lockwithvalues: <... completed>
-accountid|balance|id
----------+-------+--------
-checking |   1050|checking
-savings  |    600|savings
+accountid|balance|balance2|id
+---------+-------+--------+--------
+checking |   1050|    2100|checking
+savings  |    600|    1200|savings
 (2 rows)

 step c1: COMMIT;
 step read: SELECT * FROM accounts ORDER BY accountid;
-accountid|balance
----------+-------
-checking |   1050
-savings  |    600
+accountid|balance|balance2
+---------+-------+--------
+checking |   1050|    2100
+savings  |    600|    1200
 (2 rows)


@@ -1104,25 +1104,31 @@ subid|id
 (1 row)


-starting permutation: simplepartupdate complexpartupdate c1 c2
+starting permutation: simplepartupdate complexpartupdate c1 c2 read_part
 step simplepartupdate:
-    update parttbl set a = a;
+    update parttbl set b = b + 10;

 step complexpartupdate:
-    with u as (update parttbl set a = a returning parttbl.*)
-    update parttbl set a = u.a from u;
+    with u as (update parttbl set b = b + 1 returning parttbl.*)
+    update parttbl set b = u.b + 100 from u;
  <waiting ...>
 step c1: COMMIT;
 step complexpartupdate: <... completed>
 step c2: COMMIT;
+step read_part: SELECT * FROM parttbl ORDER BY a;
+a| b|c| d
+-+--+-+--
+1|12|1|13
+(1 row)
+

-starting permutation: simplepartupdate_route1to2 complexpartupdate_route_err1 c1 c2
+starting permutation: simplepartupdate_route1to2 complexpartupdate_route_err1 c1 c2 read_part
 step simplepartupdate_route1to2:
     update parttbl set a = 2 where c = 1 returning *;

-a|b|c
--+-+-
-2|1|1
+a|b|c|   d
+-+-+-+----
+2|1|1|1003
 (1 row)

 step complexpartupdate_route_err1:
@@ -1133,14 +1139,20 @@ step c1: COMMIT;
 step complexpartupdate_route_err1: <... completed>
 ERROR:  tuple to be locked was already moved to another partition due to concurrent update
 step c2: COMMIT;
+step read_part: SELECT * FROM parttbl ORDER BY a;
+a|b|c|   d
+-+-+-+----
+2|1|1|1003
+(1 row)
+

-starting permutation: simplepartupdate_noroute complexpartupdate_route c1 c2
+starting permutation: simplepartupdate_noroute complexpartupdate_route c1 c2 read_part
 step simplepartupdate_noroute:
     update parttbl set b = 2 where c = 1 returning *;

-a|b|c
--+-+-
-1|2|1
+a|b|c|d
+-+-+-+-
+1|2|1|3
 (1 row)

 step complexpartupdate_route:
@@ -1149,20 +1161,26 @@ step complexpartupdate_route:
  <waiting ...>
 step c1: COMMIT;
 step complexpartupdate_route: <... completed>
-a|b|c
--+-+-
-2|2|1
+a|b|c|   d
+-+-+-+----
+2|2|1|1004
 (1 row)

 step c2: COMMIT;
+step read_part: SELECT * FROM parttbl ORDER BY a;
+a|b|c|   d
+-+-+-+----
+2|2|1|1004
+(1 row)
+

-starting permutation: simplepartupdate_noroute complexpartupdate_doesnt_route c1 c2
+starting permutation: simplepartupdate_noroute complexpartupdate_doesnt_route c1 c2 read_part
 step simplepartupdate_noroute:
     update parttbl set b = 2 where c = 1 returning *;

-a|b|c
--+-+-
-1|2|1
+a|b|c|d
+-+-+-+-
+1|2|1|3
 (1 row)

 step complexpartupdate_doesnt_route:
@@ -1171,9 +1189,15 @@ step complexpartupdate_doesnt_route:
  <waiting ...>
 step c1: COMMIT;
 step complexpartupdate_doesnt_route: <... completed>
-a|b|c
--+-+-
-1|2|1
+a|b|c|d
+-+-+-+-
+1|2|1|3
 (1 row)

 step c2: COMMIT;
+step read_part: SELECT * FROM parttbl ORDER BY a;
+a|b|c|d
+-+-+-+-
+1|2|1|3
+(1 row)
+
diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec
index 4bb959504a..768f7098b9 100644
--- a/src/test/isolation/specs/eval-plan-qual.spec
+++ b/src/test/isolation/specs/eval-plan-qual.spec
@@ -6,7 +6,8 @@

 setup
 {
- CREATE TABLE accounts (accountid text PRIMARY KEY, balance numeric not null);
+ CREATE TABLE accounts (accountid text PRIMARY KEY, balance numeric not null,
+   balance2 numeric GENERATED ALWAYS AS (balance * 2) STORED);
  INSERT INTO accounts VALUES ('checking', 600), ('savings', 600);

  CREATE FUNCTION update_checking(int) RETURNS bool LANGUAGE sql AS $$
@@ -33,9 +34,12 @@ setup
  CREATE TABLE jointest AS SELECT generate_series(1,10) AS id, 0 AS data;
  CREATE INDEX ON jointest(id);

- CREATE TABLE parttbl (a int, b int, c int) PARTITION BY LIST (a);
+ CREATE TABLE parttbl (a int, b int, c int,
+   d int GENERATED ALWAYS AS (a + b) STORED) PARTITION BY LIST (a);
  CREATE TABLE parttbl1 PARTITION OF parttbl FOR VALUES IN (1);
- CREATE TABLE parttbl2 PARTITION OF parttbl FOR VALUES IN (2);
+ CREATE TABLE parttbl2 PARTITION OF parttbl
+   (d WITH OPTIONS GENERATED ALWAYS AS (a + b + 1000) STORED)
+   FOR VALUES IN (2);
  INSERT INTO parttbl VALUES (1, 1, 1);

  CREATE TABLE another_parttbl (a int, b int, c int) PARTITION BY LIST (a);
@@ -171,7 +175,7 @@ step selectresultforupdate    {
 # test for EPQ on a partitioned result table

 step simplepartupdate    {
-    update parttbl set a = a;
+    update parttbl set b = b + 10;
 }

 # test scenarios where update may cause row movement
@@ -223,8 +227,8 @@ step updateforcip3    {
 step wrtwcte    { UPDATE table_a SET value = 'tableAValue2' WHERE id = 1; }
 step wrjt    { UPDATE jointest SET data = 42 WHERE id = 7; }
 step complexpartupdate    {
-    with u as (update parttbl set a = a returning parttbl.*)
-    update parttbl set a = u.a from u;
+    with u as (update parttbl set b = b + 1 returning parttbl.*)
+    update parttbl set b = u.b + 100 from u;
 }

 step complexpartupdate_route_err1 {
@@ -273,6 +277,7 @@ setup        { BEGIN ISOLATION LEVEL READ COMMITTED; }
 step read    { SELECT * FROM accounts ORDER BY accountid; }
 step read_ext    { SELECT * FROM accounts_ext ORDER BY accountid; }
 step read_a        { SELECT * FROM table_a ORDER BY id; }
+step read_part    { SELECT * FROM parttbl ORDER BY a; }

 # this test exercises EvalPlanQual with a CTE, cf bug #14328
 step readwcte    {
@@ -353,7 +358,7 @@ permutation wrjt selectjoinforupdate c2 c1
 permutation wrjt selectresultforupdate c2 c1
 permutation wrtwcte multireadwcte c1 c2

-permutation simplepartupdate complexpartupdate c1 c2
-permutation simplepartupdate_route1to2 complexpartupdate_route_err1 c1 c2
-permutation simplepartupdate_noroute complexpartupdate_route c1 c2
-permutation simplepartupdate_noroute complexpartupdate_doesnt_route c1 c2
+permutation simplepartupdate complexpartupdate c1 c2 read_part
+permutation simplepartupdate_route1to2 complexpartupdate_route_err1 c1 c2 read_part
+permutation simplepartupdate_noroute complexpartupdate_route c1 c2 read_part
+permutation simplepartupdate_noroute complexpartupdate_doesnt_route c1 c2 read_part
diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out
index 702774d644..f5d802b9d1 100644
--- a/src/test/regress/expected/generated.out
+++ b/src/test/regress/expected/generated.out
@@ -792,6 +792,19 @@ SELECT * FROM gtest_child;
  07-15-2016 |  1 |  2
 (1 row)

+UPDATE gtest_parent SET f1 = f1 + 60;
+SELECT * FROM gtest_parent;
+     f1     | f2 | f3
+------------+----+----
+ 09-13-2016 |  1 | 33
+(1 row)
+
+SELECT * FROM gtest_child3;
+     f1     | f2 | f3
+------------+----+----
+ 09-13-2016 |  1 | 33
+(1 row)
+
 -- we leave these tables around for purposes of testing dump/reload/upgrade
 -- generated columns in partition key (not allowed)
 CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) STORED) PARTITION BY
RANGE(f3); 
diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sql
index 36f6bff348..8ddecf0cc3 100644
--- a/src/test/regress/sql/generated.sql
+++ b/src/test/regress/sql/generated.sql
@@ -413,6 +413,9 @@ ALTER TABLE gtest_parent ATTACH PARTITION gtest_child3 FOR VALUES FROM ('2016-09
 INSERT INTO gtest_parent (f1, f2) VALUES ('2016-07-15', 1);
 SELECT * FROM gtest_parent;
 SELECT * FROM gtest_child;
+UPDATE gtest_parent SET f1 = f1 + 60;
+SELECT * FROM gtest_parent;
+SELECT * FROM gtest_child3;
 -- we leave these tables around for purposes of testing dump/reload/upgrade

 -- generated columns in partition key (not allowed)

pgsql-bugs by date:

Previous
From: Tom Lane
Date:
Subject: Re: BUG #17823: Generated columns not always updated correctly
Next
From: Thomas Munro
Date:
Subject: Re: BUG #17368: Assert failed in GetSafeSnapshot() for SERIALIZABLE READ ONLY DEFERRABLE transaction