public inbox for [email protected]
help / color / mirror / Atom feedFrom: SATYANARAYANA NARLAPURAM <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column
Date: Tue, 7 Apr 2026 01:42:42 -0700
Message-ID: <CAHg+QDcd=t69gLf9yQexO07EJ2mx0Z70NFHo6h94X1EDA=hM0g@mail.gmail.com> (raw)
Hi hackers,
It appears that this is a bug where UPDATE FOR ... PORTION OF fails to
recompute GENERATED ALWAYS AS ... STORED columns whose expressions
reference the range column being narrowed. Please find the repro below.
postgres=# CREATE TABLE t (id int, valid_at int4range NOT NULL, val int,
range_len int GENERATED ALWAYS AS (upper(valid_at) - lower(valid_at))
STORED);
INSERT INTO t VALUES (1, '[1,100)', 10);
UPDATE t FOR PORTION OF valid_at FROM 30 TO 70 SET val = 99;
SELECT *, upper(valid_at) - lower(valid_at) AS expected FROM t ORDER BY
valid_at;
CREATE TABLE
INSERT 0 1
UPDATE 1
id | valid_at | val | range_len | expected
----+----------+-----+-----------+----------
1 | [1,30) | 10 | 29 | 29
1 | [30,70) | 99 | 99 | 40
1 | [70,100) | 10 | 30 | 30
(3 rows)
The updated row [30,70) retains the stale range_len = 99 from the original
[1,100) range. The leftover inserts are correct because CMD_INSERT
unconditionally recomputes all generated columns. Virtual generated columns
are not affected and are computed correctly because they're evaluated at
read time from the actual stored valid_at value.
Further looking at the code it appears, In transformForPortionOfClause(),
the range column is intentionally not added to perminfo->updatedCols. Since
the range column is absent from updatedCols, any generated stored column
whose expression depends solely on the range column (e.g., upper(valid_at)
- lower(valid_at)) is skipped. Therefore, its expression is never prepared
and never recomputed during the FPO update.
Attached a draft patch that has the test scenario and a fix to address this
issue. In ExecInitGenerated, after retrieving updatedCols, the patch
additionally checks whether the owning ModifyTableState contains an FPO
clause. If it does, the attribute number (attno) of the range column is
added to updatedCols.
Thanks,
Satya
Attachments:
[application/octet-stream] v1-0001-fpo-generated-stored-fix.patch (5.3K, 3-v1-0001-fpo-generated-stored-fix.patch)
download | inline diff:
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index dfd7b33a..c0250e65 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -475,6 +475,38 @@ ExecInitGenerated(ResultRelInfo *resultRelInfo,
else
updatedCols = NULL;
+ /*
+ * For UPDATE ... FOR PORTION OF, the range column is also being modified
+ * (narrowed via intersection), but it is not included in updatedCols
+ * because the user does not need UPDATE permission on it. We must
+ * account for it here so that generated columns referencing the range
+ * column are recomputed.
+ */
+ if (updatedCols)
+ {
+ ForPortionOfState *fpoState = resultRelInfo->ri_forPortionOf;
+
+ if (fpoState == NULL && resultRelInfo->ri_RootResultRelInfo)
+ fpoState = resultRelInfo->ri_RootResultRelInfo->ri_forPortionOf;
+ if (fpoState != NULL)
+ {
+ int rangeAttno = fpoState->fp_rangeAttno;
+
+ /* Map from root attno to child attno if needed */
+ if (resultRelInfo->ri_RootResultRelInfo)
+ {
+ TupleConversionMap *map = ExecGetRootToChildMap(resultRelInfo,
+ estate);
+
+ if (map)
+ rangeAttno = map->attrMap->attnums[rangeAttno - 1];
+ }
+
+ updatedCols = bms_add_member(bms_copy(updatedCols),
+ rangeAttno - FirstLowInvalidHeapAttributeNumber);
+ }
+ }
+
/*
* Make sure these data structures are built in the per-query memory
* context so they'll survive throughout the query.
diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out
index 31f772c7..a9917b4a 100644
--- a/src/test/regress/expected/for_portion_of.out
+++ b/src/test/regress/expected/for_portion_of.out
@@ -2097,4 +2097,62 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at;
(4 rows)
DROP TABLE temporal_partitioned;
+-- UPDATE FOR PORTION OF with generated stored columns
+-- The generated column depends on the range column, so it must be
+-- recomputed when FOR PORTION OF narrows the range.
+CREATE TABLE fpo_generated_stored (
+ id int,
+ valid_at int4range,
+ range_len int GENERATED ALWAYS AS (upper(valid_at) - lower(valid_at)) STORED
+);
+INSERT INTO fpo_generated_stored (id, valid_at) VALUES
+ (1, '[10,100)');
+SELECT * FROM fpo_generated_stored ORDER BY valid_at;
+ id | valid_at | range_len
+----+----------+-----------
+ 1 | [10,100) | 90
+(1 row)
+
+-- After FPO, all three rows (leftover-before, updated, leftover-after)
+-- must have correct range_len values.
+UPDATE fpo_generated_stored
+ FOR PORTION OF valid_at FROM 30 TO 70
+ SET id = 2;
+SELECT * FROM fpo_generated_stored ORDER BY valid_at;
+ id | valid_at | range_len
+----+----------+-----------
+ 1 | [10,30) | 20
+ 2 | [30,70) | 40
+ 1 | [70,100) | 30
+(3 rows)
+
+-- Also test with a generated column that references both a SET column
+-- and the range column.
+TRUNCATE fpo_generated_stored;
+DROP TABLE fpo_generated_stored;
+CREATE TABLE fpo_generated_stored (
+ id int,
+ valid_at int4range,
+ id_plus_len int GENERATED ALWAYS AS (id + upper(valid_at) - lower(valid_at)) STORED
+);
+INSERT INTO fpo_generated_stored (id, valid_at) VALUES
+ (1, '[10,100)');
+SELECT * FROM fpo_generated_stored ORDER BY valid_at;
+ id | valid_at | id_plus_len
+----+----------+-------------
+ 1 | [10,100) | 91
+(1 row)
+
+UPDATE fpo_generated_stored
+ FOR PORTION OF valid_at FROM 30 TO 70
+ SET id = 2;
+SELECT * FROM fpo_generated_stored ORDER BY valid_at;
+ id | valid_at | id_plus_len
+----+----------+-------------
+ 1 | [10,30) | 21
+ 2 | [30,70) | 42
+ 1 | [70,100) | 31
+(3 rows)
+
+DROP TABLE fpo_generated_stored;
RESET datestyle;
diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql
index d4062acf..f1e3937c 100644
--- a/src/test/regress/sql/for_portion_of.sql
+++ b/src/test/regress/sql/for_portion_of.sql
@@ -1365,4 +1365,44 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at;
DROP TABLE temporal_partitioned;
+-- UPDATE FOR PORTION OF with generated stored columns
+-- The generated column depends on the range column, so it must be
+-- recomputed when FOR PORTION OF narrows the range.
+
+CREATE TABLE fpo_generated_stored (
+ id int,
+ valid_at int4range,
+ range_len int GENERATED ALWAYS AS (upper(valid_at) - lower(valid_at)) STORED
+);
+INSERT INTO fpo_generated_stored (id, valid_at) VALUES
+ (1, '[10,100)');
+SELECT * FROM fpo_generated_stored ORDER BY valid_at;
+
+-- After FPO, all three rows (leftover-before, updated, leftover-after)
+-- must have correct range_len values.
+UPDATE fpo_generated_stored
+ FOR PORTION OF valid_at FROM 30 TO 70
+ SET id = 2;
+SELECT * FROM fpo_generated_stored ORDER BY valid_at;
+
+-- Also test with a generated column that references both a SET column
+-- and the range column.
+TRUNCATE fpo_generated_stored;
+DROP TABLE fpo_generated_stored;
+CREATE TABLE fpo_generated_stored (
+ id int,
+ valid_at int4range,
+ id_plus_len int GENERATED ALWAYS AS (id + upper(valid_at) - lower(valid_at)) STORED
+);
+INSERT INTO fpo_generated_stored (id, valid_at) VALUES
+ (1, '[10,100)');
+SELECT * FROM fpo_generated_stored ORDER BY valid_at;
+
+UPDATE fpo_generated_stored
+ FOR PORTION OF valid_at FROM 30 TO 70
+ SET id = 2;
+SELECT * FROM fpo_generated_stored ORDER BY valid_at;
+
+DROP TABLE fpo_generated_stored;
+
RESET datestyle;
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column
In-Reply-To: <CAHg+QDcd=t69gLf9yQexO07EJ2mx0Z70NFHo6h94X1EDA=hM0g@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox