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 122de068022edb718802a9720afa13ee1f7f4571..7284a23dc9abaf52087367acc4eaf714e65469a2 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 4c49edef25038f3a45109d81e7f6fc7ce07b13b2..5121dff7f5bbad3b52e6eeedf4aed08e6990970b 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -361,7 +361,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 c318681b9ae95c4d3fa6e58d78b2ae024e5f699e..372cec47eb3d7a440fb99c69d1df38fb70f3cfde 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 3d96f6f30f825c1bf6c1b184a3a0efe4015db815..c50824026c56969d9a6887655cea77e253fd0e21 100644 (file)
--- a/src/test/subscription/t/011_generated.pl
+++ b/src/test/subscription/t/011_generated.pl
@@ -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,11 +55,45 @@ $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');
 
 done_testing();