Optimize update of tables with generated columns

From 64ed6fcc95be87b2313ea61ca193120bce7d506c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 13 Feb 2020 14:29:40 +0100
Subject: [PATCH v2 1/2] Fill in extraUpdatedCols in logical replication

The extraUpdatedCols field of the target RTE records which generated
columns are affected by an update.  This is used in a variety of
places, including per-column triggers and foreign data wrappers.  When
an update was initiated by a logical replication subscription, this
field was not filled in, so such an update would not affect generated
columns in a way that is consistent with normal updates.  To fix,
factor out some code from analyze.c to fill in extraUpdatedCols in the
logical replication worker as well.
---
 src/backend/parser/analyze.c             | 18 ++++++++++++------
 src/backend/replication/logical/worker.c |  3 +++
 src/include/parser/analyze.h             |  2 ++
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 748bebffc1..6676412842 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2346,10 +2346,18 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist)
 	if (orig_tl != NULL)
 		elog(ERROR, "UPDATE target count mismatch --- internal error");
 
-	/*
-	 * Record in extraUpdatedCols generated columns referencing updated base
-	 * columns.
-	 */
+	fill_extraUpdatedCols(target_rte, tupdesc);
+
+	return tlist;
+}
+
+/*
+ * Record in extraUpdatedCols generated columns referencing updated base
+ * columns.
+ */
+void
+fill_extraUpdatedCols(RangeTblEntry *target_rte, TupleDesc tupdesc)
+{
 	if (tupdesc->constr &&
 		tupdesc->constr->has_generated_stored)
 	{
@@ -2371,8 +2379,6 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist)
 															  defval.adnum - FirstLowInvalidHeapAttributeNumber);
 		}
 	}
-
-	return tlist;
 }
 
 /*
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 7a5471f95c..ad4a732fd2 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -42,6 +42,7 @@
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "optimizer/optimizer.h"
+#include "parser/analyze.h"
 #include "parser/parse_relation.h"
 #include "pgstat.h"
 #include "postmaster/bgworker.h"
@@ -737,6 +738,8 @@ apply_handle_update(StringInfo s)
 													 i + 1 - FirstLowInvalidHeapAttributeNumber);
 	}
 
+	fill_extraUpdatedCols(target_rte, RelationGetDescr(rel->localrel));
+
 	PushActiveSnapshot(GetTransactionSnapshot());
 	ExecOpenIndices(estate->es_result_relation_info, false);
 
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
index d6a467a572..9d09a02141 100644
--- a/src/include/parser/analyze.h
+++ b/src/include/parser/analyze.h
@@ -46,4 +46,6 @@ extern void applyLockingClause(Query *qry, Index rtindex,
 extern List *BuildOnConflictExcludedTargetlist(Relation targetrel,
 											   Index exclRelIndex);
 
+extern void fill_extraUpdatedCols(RangeTblEntry *target_rte, TupleDesc tupdesc);
+
 #endif							/* ANALYZE_H */
-- 
2.25.0

From 017d31b53af07a5599028099e38f7422401888c2 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 20 Dec 2019 22:52:31 +0100
Subject: [PATCH v2 2/2] Optimize update of tables with generated columns

When updating a table row with generated columns, only recompute those
generated columns whose base columns have changed in this update and
keep the rest unchanged.  This can result in a significant performance
benefit.  The required information was already kept in
RangeTblEntry.extraUpdatedCols; we just have to make use of it.
---
 src/backend/commands/copy.c            |  2 +-
 src/backend/executor/execReplication.c |  4 +--
 src/backend/executor/nodeModifyTable.c | 37 +++++++++++++++++++++-----
 src/include/executor/nodeModifyTable.h |  2 +-
 src/include/nodes/execnodes.h          |  3 +++
 5 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 40a8ec1abd..e79ede4cb8 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -3222,7 +3222,7 @@ CopyFrom(CopyState cstate)
 				/* Compute stored generated columns */
 				if (resultRelInfo->ri_RelationDesc->rd_att->constr &&
 					resultRelInfo->ri_RelationDesc->rd_att->constr->has_generated_stored)
-					ExecComputeStoredGenerated(estate, myslot);
+					ExecComputeStoredGenerated(estate, myslot, CMD_INSERT);
 
 				/*
 				 * If the target is a plain table, check the constraints of
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 30cba89da7..7194becfd9 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -419,7 +419,7 @@ ExecSimpleRelationInsert(EState *estate, TupleTableSlot *slot)
 		/* Compute stored generated columns */
 		if (rel->rd_att->constr &&
 			rel->rd_att->constr->has_generated_stored)
-			ExecComputeStoredGenerated(estate, slot);
+			ExecComputeStoredGenerated(estate, slot, CMD_INSERT);
 
 		/* Check the constraints of the tuple */
 		if (rel->rd_att->constr)
@@ -485,7 +485,7 @@ ExecSimpleRelationUpdate(EState *estate, EPQState *epqstate,
 		/* Compute stored generated columns */
 		if (rel->rd_att->constr &&
 			rel->rd_att->constr->has_generated_stored)
-			ExecComputeStoredGenerated(estate, slot);
+			ExecComputeStoredGenerated(estate, slot, CMD_UPDATE);
 
 		/* Check the constraints of the tuple */
 		if (rel->rd_att->constr)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 59d1a31c97..d71c0a4322 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -246,7 +246,7 @@ ExecCheckTIDVisible(EState *estate,
  * Compute stored generated columns for a tuple
  */
 void
-ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot)
+ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot, CmdType cmdtype)
 {
 	ResultRelInfo *resultRelInfo = estate->es_result_relation_info;
 	Relation	rel = resultRelInfo->ri_RelationDesc;
@@ -269,6 +269,7 @@ ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot)
 
 		resultRelInfo->ri_GeneratedExprs =
 			(ExprState **) palloc(natts * sizeof(ExprState *));
+		resultRelInfo->ri_NumGeneratedNeeded = 0;
 
 		for (int i = 0; i < natts; i++)
 		{
@@ -276,18 +277,41 @@ ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot)
 			{
 				Expr	   *expr;
 
+				/*
+				 * If it's an update and the current column was not marked as
+				 * being updated, then we can skip the computation.  But if
+				 * there is a BEFORE ROW UPDATE trigger, we cannot skip
+				 * because the trigger might affect additional columns.
+				 */
+				if (cmdtype == CMD_UPDATE &&
+					!(rel->trigdesc && rel->trigdesc->trig_update_before_row) &&
+					!bms_is_member(i + 1 - FirstLowInvalidHeapAttributeNumber,
+								   exec_rt_fetch(resultRelInfo->ri_RangeTableIndex, estate)->extraUpdatedCols))
+				{
+					resultRelInfo->ri_GeneratedExprs[i] = NULL;
+					continue;
+				}
+
 				expr = (Expr *) build_column_default(rel, i + 1);
 				if (expr == NULL)
 					elog(ERROR, "no generation expression found for column number %d of table \"%s\"",
 						 i + 1, RelationGetRelationName(rel));
 
 				resultRelInfo->ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
+				resultRelInfo->ri_NumGeneratedNeeded++;
 			}
 		}
 
 		MemoryContextSwitchTo(oldContext);
 	}
 
+	/*
+	 * If no generated columns have been affected by this change, then skip
+	 * the rest.
+	 */
+	if (resultRelInfo->ri_NumGeneratedNeeded == 0)
+		return;
+
 	oldContext = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
 
 	values = palloc(sizeof(*values) * natts);
@@ -300,7 +324,8 @@ ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot)
 	{
 		Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
 
-		if (attr->attgenerated == ATTRIBUTE_GENERATED_STORED)
+		if (attr->attgenerated == ATTRIBUTE_GENERATED_STORED &&
+			resultRelInfo->ri_GeneratedExprs[i])
 		{
 			ExprContext *econtext;
 			Datum		val;
@@ -392,7 +417,7 @@ ExecInsert(ModifyTableState *mtstate,
 		 */
 		if (resultRelationDesc->rd_att->constr &&
 			resultRelationDesc->rd_att->constr->has_generated_stored)
-			ExecComputeStoredGenerated(estate, slot);
+			ExecComputeStoredGenerated(estate, slot, CMD_INSERT);
 
 		/*
 		 * insert into foreign table: let the FDW do it
@@ -427,7 +452,7 @@ ExecInsert(ModifyTableState *mtstate,
 		 */
 		if (resultRelationDesc->rd_att->constr &&
 			resultRelationDesc->rd_att->constr->has_generated_stored)
-			ExecComputeStoredGenerated(estate, slot);
+			ExecComputeStoredGenerated(estate, slot, CMD_INSERT);
 
 		/*
 		 * Check any RLS WITH CHECK policies.
@@ -1088,7 +1113,7 @@ ExecUpdate(ModifyTableState *mtstate,
 		 */
 		if (resultRelationDesc->rd_att->constr &&
 			resultRelationDesc->rd_att->constr->has_generated_stored)
-			ExecComputeStoredGenerated(estate, slot);
+			ExecComputeStoredGenerated(estate, slot, CMD_UPDATE);
 
 		/*
 		 * update in foreign table: let the FDW do it
@@ -1125,7 +1150,7 @@ ExecUpdate(ModifyTableState *mtstate,
 		 */
 		if (resultRelationDesc->rd_att->constr &&
 			resultRelationDesc->rd_att->constr->has_generated_stored)
-			ExecComputeStoredGenerated(estate, slot);
+			ExecComputeStoredGenerated(estate, slot, CMD_UPDATE);
 
 		/*
 		 * Check any RLS UPDATE WITH CHECK policies
diff --git a/src/include/executor/nodeModifyTable.h b/src/include/executor/nodeModifyTable.h
index 0495cae051..4ec4ebdabc 100644
--- a/src/include/executor/nodeModifyTable.h
+++ b/src/include/executor/nodeModifyTable.h
@@ -15,7 +15,7 @@
 
 #include "nodes/execnodes.h"
 
-extern void ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot);
+extern void ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot, CmdType cmdtype);
 
 extern ModifyTableState *ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags);
 extern void ExecEndModifyTable(ModifyTableState *node);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 5d5b38b879..cd3ddf781f 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -457,6 +457,9 @@ typedef struct ResultRelInfo
 	/* array of stored generated columns expr states */
 	ExprState **ri_GeneratedExprs;
 
+	/* number of stored generated columns we need to compute */
+	int			ri_NumGeneratedNeeded;
+
 	/* for removing junk attributes from tuples */
 	JunkFilter *ri_junkFilter;
 
-- 
2.25.0