Make new GENERATED-expressions code more bulletproof.

In commit 8bf6ec3ba I assumed that no code path could reach
ExecGetExtraUpdatedCols without having gone through
ExecInitStoredGenerated.  That turns out not to be the case in
logical replication: if there's an ON UPDATE trigger on the target
table, trigger.c will call this code before anybody has set up its
generated columns.  Having seen that, I don't have a lot of faith in
there not being other such paths.  ExecGetExtraUpdatedCols can call
ExecInitStoredGenerated for itself, as long as we are willing to
assume that it is only called in CMD_UPDATE operations, which on
the whole seems like a safer leap of faith.

Per report from Vitaly Davydov.

Discussion: https://postgr.es/m/d259d69652b8c2ff50e14cda3c236c7f@postgrespro.ru

diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 9705460ebda71ecb4ab5706f56102edd0c21aab4..cad3a7f9ada87be4995b5df03a9dc8bdc2050427 100644 (file)
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -52,6 +52,7 @@
 #include "access/transam.h"
 #include "executor/executor.h"
 #include "executor/execPartition.h"
+#include "executor/nodeModifyTable.h"
 #include "jit/jit.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
@@ -1332,8 +1333,9 @@ ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate)
    {
        ListCell   *lc;
 
-       /* Assert that ExecInitStoredGenerated has been called. */
-       Assert(relinfo->ri_GeneratedExprs != NULL);
+       /* In some code paths we can reach here before initializing the info */
+       if (relinfo->ri_GeneratedExprs == NULL)
+           ExecInitStoredGenerated(relinfo, estate, CMD_UPDATE);
        foreach(lc, estate->es_resultrelinfo_extra)
        {
            ResultRelInfoExtra *rextra = (ResultRelInfoExtra *) lfirst(lc);

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index d92bbd1c8453e3dba7f51593872538c58b6a9f0b..078055424663e57431c4166b2a5e59d401459afb 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -260,7 +260,7 @@ ExecCheckTIDVisible(EState *estate,
  * (Currently, ri_extraUpdatedCols is consulted only in UPDATE, but we might
  * as well fill it for INSERT too.)
  */
-static void
+void
 ExecInitStoredGenerated(ResultRelInfo *resultRelInfo,
                        EState *estate,
                        CmdType cmdtype)

diff --git a/src/include/executor/nodeModifyTable.h b/src/include/executor/nodeModifyTable.h
index 83e296553104f4a2fe5bd99d90b96ca79ea404ac..7b20f14b72ed165b89cc694f86eb421cad1cc9e9 100644 (file)
--- a/src/include/executor/nodeModifyTable.h
+++ b/src/include/executor/nodeModifyTable.h
@@ -15,6 +15,10 @@
 
 #include "nodes/execnodes.h"
 
+extern void ExecInitStoredGenerated(ResultRelInfo *resultRelInfo,
+                                   EState *estate,
+                                   CmdType cmdtype);
+
 extern void ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
                                       EState *estate, TupleTableSlot *slot,
                                       CmdType cmdtype);

diff --git a/src/test/subscription/t/011_generated.pl b/src/test/subscription/t/011_generated.pl
index 0662c55f082ec235801398f5f0f6c5ed09b54e08..3a92b7187761cbf7709cb988008eb61dc4284da6 100644 (file)
--- a/src/test/subscription/t/011_generated.pl
+++ b/src/test/subscription/t/011_generated.pl
@@ -6,7 +6,7 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 2;
+use Test::More tests => 3;
 
 # setup
 
@@ -25,7 +25,7 @@ $node_publisher->safe_psql('postgres',
 );
 
 $node_subscriber->safe_psql('postgres',
-   "CREATE TABLE tab1 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 22) STORED)"
+   "CREATE TABLE tab1 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 22) STORED, c int)"
 );
 
 # data for initial sync
@@ -55,9 +55,43 @@ $node_publisher->safe_psql('postgres', "UPDATE tab1 SET a = 6 WHERE a = 5");
 
 $node_publisher->wait_for_catchup('sub1');
 
-$result = $node_subscriber->safe_psql('postgres', "SELECT a, b FROM tab1");
-is( $result, qq(1|22
-2|44
-3|66
-4|88
-6|132), 'generated columns replicated');
+$result = $node_subscriber->safe_psql('postgres', "SELECT * FROM tab1");
+is( $result, qq(1|22|
+2|44|
+3|66|
+4|88|
+6|132|), 'generated columns replicated');
+
+# try it with a subscriber-side trigger
+
+$node_subscriber->safe_psql(
+   'postgres', q{
+CREATE FUNCTION tab1_trigger_func() RETURNS trigger
+LANGUAGE plpgsql AS $$
+BEGIN
+  NEW.c := NEW.a + 10;
+  RETURN NEW;
+END $$;
+
+CREATE TRIGGER test1 BEFORE INSERT OR UPDATE ON tab1
+  FOR EACH ROW
+  EXECUTE PROCEDURE tab1_trigger_func();
+
+ALTER TABLE tab1 ENABLE REPLICA TRIGGER test1;
+});
+
+$node_publisher->safe_psql('postgres', "INSERT INTO tab1 VALUES (7), (8)");
+
+$node_publisher->safe_psql('postgres', "UPDATE tab1 SET a = 9 WHERE a = 7");
+
+$node_publisher->wait_for_catchup('sub1');
+
+$result =
+  $node_subscriber->safe_psql('postgres', "SELECT * FROM tab1 ORDER BY 1");
+is( $result, qq(1|22|
+2|44|
+3|66|
+4|88|
+6|132|
+8|176|18
+9|198|19), 'generated columns replicated with trigger');