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
This commit is contained in:
Tom Lane 2023-01-15 13:14:52 -05:00
parent 1334b79a35
commit 3f244d020f
4 changed files with 50 additions and 17 deletions

View File

@ -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"
@ -1342,15 +1343,9 @@ ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate)
Bitmapset *
ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate)
{
#ifdef USE_ASSERT_CHECKING
/* Verify that ExecInitStoredGenerated has been called if needed. */
Relation rel = relinfo->ri_RelationDesc;
TupleDesc tupdesc = RelationGetDescr(rel);
if (tupdesc->constr && tupdesc->constr->has_generated_stored)
Assert(relinfo->ri_GeneratedExprs != NULL);
#endif
/* In some code paths we can reach here before initializing the info */
if (relinfo->ri_GeneratedExprs == NULL)
ExecInitStoredGenerated(relinfo, estate, CMD_UPDATE);
return relinfo->ri_extraUpdatedCols;
}

View File

@ -360,7 +360,7 @@ ExecCheckTIDVisible(EState *estate,
* fields. (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)

View File

@ -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);

View File

@ -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();