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

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: SATYANARAYANA NARLAPURAM <satyanarlapuram(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 09:10:30
Message-ID: CACJufxGreOtA-S-qeHyS5iSSsj5zZX0W3Rf8FxbyL+SVXFjLYw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

--
jian
https://www.enterprisedb.com/

Attachment Content-Type Size
v2-0001-FOR-PORTION-OF-UPDATE-misc.patch text/x-patch 21.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Erik Rijkers 2026-04-10 09:18:01 pg_stash_advice doc
Previous Message Henson Choi 2026-04-10 08:51:54 Re: Row pattern recognition