Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column - Mailing list pgsql-hackers
| From | SATYANARAYANA NARLAPURAM |
|---|---|
| Subject | Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column |
| Date | |
| Msg-id | CAHg+QDeGLfz8YSCChjqrxaVSrz9AnMA0NrmsNogLqeGgCt7-wg@mail.gmail.com Whole thread |
| In response to | Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column (jian he <jian.universality@gmail.com>) |
| Responses |
Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column
Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column |
| List | pgsql-hackers |
Hi Jian,
On Fri, Apr 10, 2026 at 2:11 AM jian he <jian.universality@gmail.com> wrote:
Hi.
ExecUpdate->ExecUpdateEpilogue->ExecForPortionOfLeftovers
In ExecForPortionOfLeftovers, we have
"""
if (!resultRelInfo->ri_forPortionOf)
{
/*
* If we don't have a ForPortionOfState yet, we must be a partition
* child being hit for the first time. Make a copy from the root, with
* our own tupleTableSlot. We do this lazily so that we don't pay the
* price of unused partitions.
*/
ForPortionOfState *leafState = makeNode(ForPortionOfState);
}
"""
We reached the end of ExecUpdate, and then suddenly initialized
resultRelInfo->ri_forPortionOf.
That seems wrong; we should initialize resultRelInfo->ri_forPortionOf
earlier so other places can use that information, such as
ForPortionOfState->fp_rangeAttno.
We can initialize ForPortionOfState right after ExecModifyTable:
"""
/* If it's not the same as last time, we need to locate the rel */
if (resultoid != node->mt_lastResultOid)
resultRelInfo = ExecLookupResultRelByOid(node, resultoid,
false, true);
"""
In ExecForPortionOfLeftovers, we should use ForPortionOfState more and
ForPortionOfExpr less.
(ForPortionOfExpr and ForPortionOfState share some overlapping information;
maybe we can eliminate some common fields or put ForPortionOfExpr into
ForPortionOfState).
As noted in [1], the FOR PORTION OF column is physically modified,
even though we didn't require explicit UPDATE privileges,
we failed to track this column in ExecGetUpdatedCols and
ExecGetExtraUpdatedCols.
This omission directly impacts the ExecInsertIndexTuples ->
index_unchanged_by_update -> ExecGetExtraUpdatedCols execution path.
We should ensure ExecGetExtraUpdatedCols also accounts for this column.
Otherwise, we need a clearer explanation for why
index_unchanged_by_update can safely ignore a column that is being
physically modified.
I have added regression test cases for CREATE TRIGGER UPDATE OF column_name.
The attached patch also addressed the table inheritance issue in
https://postgr.es/m/CAHg+QDcsXsUVaZ+JwM02yDRQEi=cL_rTH_ROLDYgOx004sQu7A@mail.gmail.com
I've combined all these changes into a single patch for now, as they
seem closely related.
[1]: https://postgr.es/m/CACJufxHALFKca5SMn5DNnbrX2trPamVL6napn_nm35p15yw+rg@mail.gmail.com
I applied your patch and tested. The following scenarios are now passing: (1) table inheritance issue I reported in [1], (2) issue reported in this thread.
Following are still failing:
(1) instead of triggers + views, mentioned in the thread [2], it has both the test case and the fix.
(2) For Portion Of DELETE loses rows when a BEFORE INSERT trigger returns NULL
DROP TABLE IF EXISTS subscriptions CASCADE;
CREATE TABLE subscriptions (
sub_id int,
period int4range NOT NULL,
plan text
);
CREATE OR REPLACE FUNCTION reject_new_subscriptions() RETURNS trigger AS $$
BEGIN
-- Business rule: no new subscription rows allowed via INSERT.
RETURN NULL;
END;
$$ LANGUAGE plpgsql;
CREATE TRIGGER no_new_subs
BEFORE INSERT ON subscriptions
FOR EACH ROW EXECUTE FUNCTION reject_new_subscriptions();
-- Pre-existing row (bypass trigger to seed it).
ALTER TABLE subscriptions DISABLE TRIGGER no_new_subs;
INSERT INTO subscriptions VALUES (1, '[1,100)', 'premium');
ALTER TABLE subscriptions ENABLE TRIGGER no_new_subs;
SELECT * FROM subscriptions;
-- 1 row: (1, [1,100), premium)
-- Delete just the [40,60) slice.
DELETE FROM subscriptions FOR PORTION OF period FROM 40 TO 60;
SELECT * FROM subscriptions ORDER BY period;
-- Should be two rows: [1,40) and [60,100)
-- Actually: 0 rows. The whole subscription vanished.
SELECT count(*) AS remaining FROM subscriptions;
-- Expected 2, got 0.
CREATE TABLE subscriptions (
sub_id int,
period int4range NOT NULL,
plan text
);
CREATE OR REPLACE FUNCTION reject_new_subscriptions() RETURNS trigger AS $$
BEGIN
-- Business rule: no new subscription rows allowed via INSERT.
RETURN NULL;
END;
$$ LANGUAGE plpgsql;
CREATE TRIGGER no_new_subs
BEFORE INSERT ON subscriptions
FOR EACH ROW EXECUTE FUNCTION reject_new_subscriptions();
-- Pre-existing row (bypass trigger to seed it).
ALTER TABLE subscriptions DISABLE TRIGGER no_new_subs;
INSERT INTO subscriptions VALUES (1, '[1,100)', 'premium');
ALTER TABLE subscriptions ENABLE TRIGGER no_new_subs;
SELECT * FROM subscriptions;
-- 1 row: (1, [1,100), premium)
-- Delete just the [40,60) slice.
DELETE FROM subscriptions FOR PORTION OF period FROM 40 TO 60;
SELECT * FROM subscriptions ORDER BY period;
-- Should be two rows: [1,40) and [60,100)
-- Actually: 0 rows. The whole subscription vanished.
SELECT count(*) AS remaining FROM subscriptions;
-- Expected 2, got 0.
(3) FPO UPDATE loses leftovers the same way
DROP TABLE IF EXISTS room_bookings CASCADE;
CREATE TABLE room_bookings (
booking_id int,
slot int4range NOT NULL,
note text
);
CREATE OR REPLACE FUNCTION block_booking_inserts() RETURNS trigger AS $$
BEGIN
-- Business rule: bookings created only through an API layer.
RETURN NULL;
END;
$$ LANGUAGE plpgsql;
CREATE TRIGGER booking_guard
BEFORE INSERT ON room_bookings
FOR EACH ROW EXECUTE FUNCTION block_booking_inserts();
ALTER TABLE room_bookings DISABLE TRIGGER booking_guard;
INSERT INTO room_bookings VALUES (1, '[1,100)', 'team meeting');
ALTER TABLE room_bookings ENABLE TRIGGER booking_guard;
SELECT * FROM room_bookings;
-- 1 row: (1, [1,100), team meeting)
-- Shorten the meeting to only [40,60).
UPDATE room_bookings FOR PORTION OF slot FROM 40 TO 60 SET note = 'shortened';
SELECT * FROM room_bookings ORDER BY slot;
-- Should be three rows:
-- [1,40) team meeting
-- [40,60) shortened
-- [60,100) team meeting
-- Actually: only the [40,60) row survives.
SELECT count(*) AS remaining FROM room_bookings;
-- Expected 3, got 1.
CREATE TABLE room_bookings (
booking_id int,
slot int4range NOT NULL,
note text
);
CREATE OR REPLACE FUNCTION block_booking_inserts() RETURNS trigger AS $$
BEGIN
-- Business rule: bookings created only through an API layer.
RETURN NULL;
END;
$$ LANGUAGE plpgsql;
CREATE TRIGGER booking_guard
BEFORE INSERT ON room_bookings
FOR EACH ROW EXECUTE FUNCTION block_booking_inserts();
ALTER TABLE room_bookings DISABLE TRIGGER booking_guard;
INSERT INTO room_bookings VALUES (1, '[1,100)', 'team meeting');
ALTER TABLE room_bookings ENABLE TRIGGER booking_guard;
SELECT * FROM room_bookings;
-- 1 row: (1, [1,100), team meeting)
-- Shorten the meeting to only [40,60).
UPDATE room_bookings FOR PORTION OF slot FROM 40 TO 60 SET note = 'shortened';
SELECT * FROM room_bookings ORDER BY slot;
-- Should be three rows:
-- [1,40) team meeting
-- [40,60) shortened
-- [60,100) team meeting
-- Actually: only the [40,60) row survives.
SELECT count(*) AS remaining FROM room_bookings;
-- Expected 3, got 1.
Thanks,
Satya
pgsql-hackers by date: