Hi Jian,

On Fri, Apr 10, 2026 at 2:11 AM jian he <[email protected]> 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_roldygox004sq...@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...@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_roldygox004sq...@mail.gmail.com
[2]:
https://www.postgresql.org/message-id/flat/CACJufxHALFKca5SMn5DNnbrX2trPamVL6napn_nm35p15yw%2Brg%40mail.gmail.com#ab4216dc6828fb0d7ada48aab9e330d0

Thanks,
Satya

Reply via email to