Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column

From: SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column
Date: 2026-04-10 22:00:57
Message-ID: CAHg+QDeGLfz8YSCChjqrxaVSrz9AnMA0NrmsNogLqeGgCt7-wg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Jian,

On Fri, Apr 10, 2026 at 2:11 AM jian he <jian(dot)universality(at)gmail(dot)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.

(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.

[1]:
https://postgr.es/m/CAHg+QDcsXsUVaZ+JwM02yDRQEi=cL_rTH_ROLDYgOx004sQu7A@mail.gmail.com
[2]:
https://www.postgresql.org/message-id/flat/CACJufxHALFKca5SMn5DNnbrX2trPamVL6napn_nm35p15yw%2Brg%40mail.gmail.com#ab4216dc6828fb0d7ada48aab9e330d0

Thanks,
Satya

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Haibo Yan 2026-04-10 23:34:21 Re: Eliminating SPI / SQL from some RI triggers - take 3
Previous Message Haibo Yan 2026-04-10 21:48:32 Re: Extract numeric filed in JSONB more effectively