On 2019-12-21 07:47, Peter Eisentraut wrote:
When updating a table row with generated columns, we only need to
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 (easy to reproduce for example with a
tsvector column). The required information was already kept in
RangeTblEntry.extraUpdatedCols; we just have to make use of it.
A small problem is that right now ExecSimpleRelationUpdate() does not
populate extraUpdatedCols. That needs fixing first.
Here is an updated patch set that contains a fix for the issue above
(should be backpatched IMO) and the actual performance patch as before.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 64ed6fcc95be87b2313ea61ca193120bce7d506c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@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 <pe...@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
