public inbox for [email protected]
help / color / mirror / Atom feedFrom: jian he <[email protected]>
To: Paul A Jungwirth <[email protected]>
Cc: SATYANARAYANA NARLAPURAM <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column
Date: Fri, 17 Apr 2026 16:13:28 +0800
Message-ID: <CACJufxHYntqy2fo9CFWDDrqKjcMK8DGRM3kse4YnXYnPYq2Hiw@mail.gmail.com> (raw)
In-Reply-To: <CA+renyWD+XXifwswE74vhjooqbiVKu4qVhLvpMcUQBzrjVjT7A@mail.gmail.com>
References: <CAHg+QDcd=t69gLf9yQexO07EJ2mx0Z70NFHo6h94X1EDA=hM0g@mail.gmail.com>
<CACJufxGreOtA-S-qeHyS5iSSsj5zZX0W3Rf8FxbyL+SVXFjLYw@mail.gmail.com>
<CAHg+QDeGLfz8YSCChjqrxaVSrz9AnMA0NrmsNogLqeGgCt7-wg@mail.gmail.com>
<CA+renyWD+XXifwswE74vhjooqbiVKu4qVhLvpMcUQBzrjVjT7A@mail.gmail.com>
On Thu, Apr 16, 2026 at 4:59 AM Paul A Jungwirth
<[email protected]> wrote:
>
> There's something I think we could still improve: we omit the
> valid-time in updatedCols, since that bitmapset is for permissions
> checking (at least originally), but now other features are using it as
> well. Our fix adds special logic to consider the valid-at column for
> GENERATED column dependencies and more special logic for UPDATE OF
> triggers. Perhaps we should add the attno to updatedCols after all,
> and put the special logic in the permissions check instead? That seems
> simpler and more robust. Or maybe it's time to have separate
> bitmapsets, one for permissions and another for everything else? What
> do you think?
>
+ /*
+ * For UPDATE ... FOR PORTION OF, the range column is also being modified
+ * (narrowed via intersection), but it is not included in updatedCols
+ * because the user does not need UPDATE permission on it. We must
+ * account for it here so that generated columns referencing the range
+ * column are recomputed.
+ */
+ if (resultRelInfo->ri_forPortionOf)
+ {
+ AttrNumber rangeAttno = resultRelInfo->ri_forPortionOf->fp_rangeAttno;
+
+ updatedCols = bms_add_member(bms_copy(updatedCols),
+ rangeAttno - FirstLowInvalidHeapAttributeNumber);
+ }
+
Putting the above into ExecGetUpdatedCols would be more neat.
InitPlan->ExecCheckPermissions happened earlier than ExecGetUpdatedCols,
So I think it will work.
Another reason to do it this way is that some places only call
ExecGetUpdatedCols, not ExecGetExtraUpdatedCols.
Put the above into ExecGetUpdatedCols, then we don't need to worry
about whether ExecGetExtraUpdatedCols is called.
ExecGetExtraUpdatedCols saves the ri_extraUpdatedCols to estate->es_query_cxt.
For ExecGetUpdatedCols, we can do the same: save the FOR PORTION OF
column to estate->es_query_cxt.
Please find the attached diff, which is based on your V3 patch.
ExecForPortionOfLeftovers
/*
* Get the range's type cache entry. This is worth caching for the whole
* UPDATE/DELETE as range functions do.
*/
typcache = fpoState->fp_leftoverstypcache;
if (typcache == NULL)
{
typcache = lookup_type_cache(forPortionOf->rangeType, 0);
fpoState->fp_leftoverstypcache = typcache;
}
It seems fp_leftoverstypcache is never being used?
place it to ExecInitModifyTable would be better, i think.
/*
* Get the ranges to the left/right of the targeted range. We call a SETOF
* support function and insert as many temporal leftovers as it gives us.
* Although rangetypes have 0/1/2 leftovers, multiranges have 0/1, and
* other types may have more.
*/
Currently, we only support anymultirange and anyrange. so here
"and other types may have more." is kind of wrong?
--
jian
https://www.enterprisedb.com/
Attachments:
[application/octet-stream] v4-0001-add-UPDATE-FOR-PORTION-OF-col-to-ExecGetUpdatedCols.no-cfbot (3.7K, 2-v4-0001-add-UPDATE-FOR-PORTION-OF-col-to-ExecGetUpdatedCols.no-cfbot)
download
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column
In-Reply-To: <CACJufxHYntqy2fo9CFWDDrqKjcMK8DGRM3kse4YnXYnPYq2Hiw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox