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