v10-0001-overwritten-virtual-generated-column-value-to.no-cfbot
application/octet-stream
Filename: v10-0001-overwritten-virtual-generated-column-value-to.no-cfbot
Type: application/octet-stream
Part: 0
Message:
Re: Virtual generated columns
From 5ad183eab1525a58a0bf20fb798fec8ef5d90893 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Wed, 11 Dec 2024 14:45:52 +0800
Subject: [PATCH v10 1/1] overwritten virtual generated column value to null in
trigger
overwritten trigger assign values for virtual generated column to null.
so virtual and stored generated column trigger bahavior is more aligned.
---
doc/src/sgml/trigger.sgml | 2 +-
src/backend/commands/trigger.c | 77 ++++++++++---------
src/pl/plpgsql/src/pl_exec.c | 4 +-
.../regress/expected/generated_virtual.out | 21 ++---
src/test/regress/sql/generated_virtual.sql | 8 +-
5 files changed, 61 insertions(+), 51 deletions(-)
diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml
index 46c6c2f126..3a91eae156 100644
--- a/doc/src/sgml/trigger.sgml
+++ b/doc/src/sgml/trigger.sgml
@@ -287,7 +287,7 @@
higher-level programming language should prevent access to a stored
generated column in the <literal>NEW</literal> row in a
<literal>BEFORE</literal> trigger. Changes to the value of a generated
- column in a <literal>BEFORE</literal> trigger are ignored and will be
+ column (stored or virtual) in a <literal>BEFORE</literal> trigger are ignored and will be
overwritten.
Virtual generated columns are never computed when triggers fire. In the C
language interface, their content is undefined in a trigger function.
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 473541a2d2..d304ac8072 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -102,7 +102,6 @@ static void AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
bool is_crosspart_update);
static void AfterTriggerEnlargeQueryState(void);
static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType);
-static void check_modified_virtual_generated(TupleDesc tupdesc, HeapTuple tuple);
/*
@@ -2506,7 +2505,26 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
}
else if (newtuple != oldtuple)
{
- check_modified_virtual_generated(RelationGetDescr(relinfo->ri_RelationDesc), newtuple);
+ TupleDesc tupdesc = RelationGetDescr(relinfo->ri_RelationDesc);
+ /*
+ * set virtual generated columns value to null if them are not null.
+ * see ExecBRUpdateTriggers comments. also
+ */
+ if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
+ {
+ Datum replValues = 0;
+ bool replIsnull = true;
+ for (int j = 1; j <= tupdesc->natts; j++)
+ {
+ if ((TupleDescAttr(tupdesc, j-1)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ && !heap_attisnull(newtuple, j, tupdesc))
+ newtuple = heap_modify_tuple_by_cols(newtuple, tupdesc,
+ 1,
+ &j,
+ &replValues,
+ &replIsnull);
+ }
+ }
ExecForceStoreHeapTuple(newtuple, slot, false);
@@ -3065,7 +3083,27 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
}
else if (newtuple != oldtuple)
{
- check_modified_virtual_generated(RelationGetDescr(relinfo->ri_RelationDesc), newtuple);
+ TupleDesc tupdesc = RelationGetDescr(relinfo->ri_RelationDesc);
+ /*
+ * virtual generated columns have no storage, them will not insert to the
+ * table, so if any trigger set them to not-null value, set it to
+ * null again. this apply to ExecBRInsertTriggers also.
+ */
+ if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
+ {
+ Datum replValues = 0;
+ bool replIsnull = true;
+ for (int j = 1; j <= tupdesc->natts; j++)
+ {
+ if ((TupleDescAttr(tupdesc, j-1)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ && !heap_attisnull(newtuple, j, tupdesc))
+ newtuple = heap_modify_tuple_by_cols(newtuple, tupdesc,
+ 1,
+ &j,
+ &replValues,
+ &replIsnull);
+ }
+ }
ExecForceStoreHeapTuple(newtuple, newslot, false);
@@ -6609,36 +6647,3 @@ pg_trigger_depth(PG_FUNCTION_ARGS)
{
PG_RETURN_INT32(MyTriggerDepth);
}
-
-/*
- * Check whether a trigger modified a virtual generated column and error if
- * so.
- *
- * We need to check this so that we don't end up storing a non-null value in a
- * virtual generated column.
- *
- * We don't need to check for stored generated columns, since those will be
- * overwritten later anyway.
- *
- * Alternatively, we could fix erroneous tuples here and be silent about it.
- * This would yield the same user-facing behavior for virtual and stored
- * generated columns. But it seems more complicated and not very useful in
- * practice.
- */
-static void
-check_modified_virtual_generated(TupleDesc tupdesc, HeapTuple tuple)
-{
- if (!(tupdesc->constr && tupdesc->constr->has_generated_virtual))
- return;
-
- for (int i = 0; i < tupdesc->natts; i++)
- {
- if (TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
- {
- if (!heap_attisnull(tuple, i + 1, tupdesc))
- ereport(ERROR,
- (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
- errmsg("trigger modified virtual generated column value")));
- }
- }
-}
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index e31206e7f4..9bb7d6621a 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -992,12 +992,14 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
* still contains the old value.) Alternatively, we could construct a
* whole new row structure without the generated columns, but this way
* seems more efficient and potentially less confusing.
+ * we also make virtual generated columns be null in the NEW row.
*/
if (tupdesc->constr && tupdesc->constr->has_generated_stored &&
TRIGGER_FIRED_BEFORE(trigdata->tg_event))
{
for (int i = 0; i < tupdesc->natts; i++)
- if (TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_STORED)
+ if (TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_STORED ||
+ TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
expanded_record_set_field_internal(rec_new->erh,
i + 1,
(Datum) 0,
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index 042e184ea0..7dbda2b67d 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1333,24 +1333,27 @@ BEGIN
RETURN NEW;
END;
$$;
-CREATE TRIGGER gtest12_01 BEFORE UPDATE ON gtest26
+CREATE TRIGGER gtest12_01 BEFORE UPDATE OR INSERT ON gtest26
FOR EACH ROW
EXECUTE PROCEDURE gtest_trigger_func();
-CREATE TRIGGER gtest12_02 BEFORE UPDATE ON gtest26
+CREATE TRIGGER gtest12_02 BEFORE UPDATE OR INSERT ON gtest26
FOR EACH ROW
EXECUTE PROCEDURE gtest_trigger_func4();
-CREATE TRIGGER gtest12_03 BEFORE UPDATE ON gtest26
+CREATE TRIGGER gtest12_03 BEFORE UPDATE OR INSERT ON gtest26
FOR EACH ROW
EXECUTE PROCEDURE gtest_trigger_func();
INSERT INTO gtest26 (a) VALUES (1);
-UPDATE gtest26 SET a = 11 WHERE a = 1;
-INFO: gtest12_01: BEFORE: old = (1,)
+INFO: gtest12_01: BEFORE: new = (1,)
+INFO: gtest12_03: BEFORE: new = (10,)
+UPDATE gtest26 SET a = 11 WHERE a = 10;
+INFO: gtest12_01: BEFORE: old = (10,)
INFO: gtest12_01: BEFORE: new = (11,)
-ERROR: trigger modified virtual generated column value
+INFO: gtest12_03: BEFORE: old = (10,)
+INFO: gtest12_03: BEFORE: new = (10,)
SELECT * FROM gtest26 ORDER BY a;
- a | b
----+---
- 1 | 2
+ a | b
+----+----
+ 10 | 20
(1 row)
-- LIKE INCLUDING GENERATED and dropped column handling
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index f97060ead4..2b0fc159af 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -695,20 +695,20 @@ BEGIN
END;
$$;
-CREATE TRIGGER gtest12_01 BEFORE UPDATE ON gtest26
+CREATE TRIGGER gtest12_01 BEFORE UPDATE OR INSERT ON gtest26
FOR EACH ROW
EXECUTE PROCEDURE gtest_trigger_func();
-CREATE TRIGGER gtest12_02 BEFORE UPDATE ON gtest26
+CREATE TRIGGER gtest12_02 BEFORE UPDATE OR INSERT ON gtest26
FOR EACH ROW
EXECUTE PROCEDURE gtest_trigger_func4();
-CREATE TRIGGER gtest12_03 BEFORE UPDATE ON gtest26
+CREATE TRIGGER gtest12_03 BEFORE UPDATE OR INSERT ON gtest26
FOR EACH ROW
EXECUTE PROCEDURE gtest_trigger_func();
INSERT INTO gtest26 (a) VALUES (1);
-UPDATE gtest26 SET a = 11 WHERE a = 1;
+UPDATE gtest26 SET a = 11 WHERE a = 10;
SELECT * FROM gtest26 ORDER BY a;
-- LIKE INCLUDING GENERATED and dropped column handling
--
2.34.1