| 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
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-FOR-PORTION-OF-UPDATE-misc.patch | text/x-patch | 21.6 KB |
| 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 |