MIME-Version: 1.0
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>
In-Reply-To: 
 <CA+renyWD+XXifwswE74vhjooqbiVKu4qVhLvpMcUQBzrjVjT7A@mail.gmail.com>
From: jian he <jian.universality@gmail.com>
Date: Fri, 17 Apr 2026 16:13:28 +0800
Message-ID: 
 <CACJufxHYntqy2fo9CFWDDrqKjcMK8DGRM3kse4YnXYnPYq2Hiw@mail.gmail.com>
Subject: Re: FOR PORTION OF does not recompute GENERATED STORED columns that
 depend on the range column
To: Paul A Jungwirth <pj@illuminatedcomputing.com>
Cc: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="0000000000004f9560064fa38891"
Archived-At: 
 <https://www.postgresql.org/message-id/CACJufxHYntqy2fo9CFWDDrqKjcMK8DGRM3kse4YnXYnPYq2Hiw%40mail.gmail.com>
Precedence: bulk

--0000000000004f9560064fa38891
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 16, 2026 at 4:59=E2=80=AFAM Paul A Jungwirth
<pj@illuminatedcomputing.com> 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 =3D resultRelInfo->ri_forPortionOf->fp_rangeAttno;
+
+ updatedCols =3D 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_c=
xt.
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 whol=
e
     * UPDATE/DELETE as range functions do.
     */
    typcache =3D fpoState->fp_leftoverstypcache;
    if (typcache =3D=3D NULL)
    {
        typcache =3D lookup_type_cache(forPortionOf->rangeType, 0);
        fpoState->fp_leftoverstypcache =3D 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 SE=
TOF
     * support function and insert as many temporal leftovers as it gives u=
s.
     * 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/

--0000000000004f9560064fa38891
Content-Type: application/octet-stream;
	name="v4-0001-add-UPDATE-FOR-PORTION-OF-col-to-ExecGetUpdatedCols.no-cfbot"
Content-Disposition: attachment;
	filename="v4-0001-add-UPDATE-FOR-PORTION-OF-col-to-ExecGetUpdatedCols.no-cfbot"
Content-Transfer-Encoding: base64
Content-ID: <f_mo2mro3r0>
X-Attachment-Id: f_mo2mro3r0

RnJvbSAyMjIwNTZlYzBjZTAzNzRmY2FhOWY2NGFjNzNlYzA4OTZhYzk3OWMyIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBqaWFuIGhlIDxqaWFuLnVuaXZlcnNhbGl0eUBnbWFpbC5jb20+
CkRhdGU6IEZyaSwgMTcgQXByIDIwMjYgMTY6MDM6NDQgKzA4MDAKU3ViamVjdDogW1BBVENIIHY0
IDEvMV0gYWRkIFVQREFURSBGT1IgUE9SVElPTiBPRiBjb2wgdG8gRXhlY0dldFVwZGF0ZWRDb2xz
CgpEaXNjdXNzaW9uOiBodHRwczovL3Bvc3Rnci5lcy9tL0NBSGcrUURjZD10NjlnTGY5eVFleE8w
N0VKMm14MFo3ME5GSG82aDk0WDFFREE9aE0wZ0BtYWlsLmdtYWlsLmNvbQpEaXNjdXNzaW9uOiBo
dHRwczovL3Bvc3Rnci5lcy9tL0NBSGcrUURjc1hzVVZhWitKd00wMnlEUlFFaT1jTF9yVEhfUk9M
RFlnT3gwMDRzUXU3QUBtYWlsLmdtYWlsLmNvbQotLS0KIHNyYy9iYWNrZW5kL2V4ZWN1dG9yL2V4
ZWNVdGlscy5jICAgICAgIHwgNDcgKysrKysrKysrKysrKystLS0tLS0tLS0tLS0KIHNyYy9iYWNr
ZW5kL2V4ZWN1dG9yL25vZGVNb2RpZnlUYWJsZS5jIHwgMTUgLS0tLS0tLS0KIDIgZmlsZXMgY2hh
bmdlZCwgMjUgaW5zZXJ0aW9ucygrKSwgMzcgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvc3Jj
L2JhY2tlbmQvZXhlY3V0b3IvZXhlY1V0aWxzLmMgYi9zcmMvYmFja2VuZC9leGVjdXRvci9leGVj
VXRpbHMuYwppbmRleCA1ZGY3ZjJlZGY4NS4uMmMwYWIxYzAyMTQgMTAwNjQ0Ci0tLSBhL3NyYy9i
YWNrZW5kL2V4ZWN1dG9yL2V4ZWNVdGlscy5jCisrKyBiL3NyYy9iYWNrZW5kL2V4ZWN1dG9yL2V4
ZWNVdGlscy5jCkBAIC0xNDEyLDYgKzE0MTIsMzEgQEAgRXhlY0dldFVwZGF0ZWRDb2xzKFJlc3Vs
dFJlbEluZm8gKnJlbGluZm8sIEVTdGF0ZSAqZXN0YXRlKQogCWlmIChwZXJtaW5mbyA9PSBOVUxM
KQogCQlyZXR1cm4gTlVMTDsKIAorCS8qCisJICogRm9yIFVQREFURSAuLi4gRk9SIFBPUlRJT04g
T0YsIHRoZSByYW5nZSBjb2x1bW4gaXMgYmVpbmcgbW9kaWZpZWQKKwkgKiAobmFycm93ZWQgdmlh
IGludGVyc2VjdGlvbiksIGJ1dCBpdCBpcyBub3QgaW5jbHVkZWQgaW4gdXBkYXRlZENvbHMKKwkg
KiBiZWNhdXNlIHRoZSB1c2VyIGRvZXMgbm90IG5lZWQgVVBEQVRFIHBlcm1pc3Npb24gb24gaXQu
IE5vdyBtYW5udWFseQorCSAqIGFkZCBpdCB0byB1cGRhdGVkQ29scy4KKwkgKi8KKwlpZiAocmVs
aW5mby0+cmlfZm9yUG9ydGlvbk9mKQorCXsKKwkJQXR0ck51bWJlcglyYW5nZUF0dG5vID0gcmVs
aW5mby0+cmlfZm9yUG9ydGlvbk9mLT5mcF9yYW5nZUF0dG5vOworCisJCWlmICghYm1zX2lzX21l
bWJlcihyYW5nZUF0dG5vIC0gRmlyc3RMb3dJbnZhbGlkSGVhcEF0dHJpYnV0ZU51bWJlciwKKwkJ
CQkJCSAgIHBlcm1pbmZvLT51cGRhdGVkQ29scykpCisJCXsKKwkJCU1lbW9yeUNvbnRleHQgb2xk
Q29udGV4dDsKKworCQkJb2xkQ29udGV4dCA9IE1lbW9yeUNvbnRleHRTd2l0Y2hUbyhlc3RhdGUt
PmVzX3F1ZXJ5X2N4dCk7CisKKwkJCXBlcm1pbmZvLT51cGRhdGVkQ29scyA9CisJCQkJYm1zX2Fk
ZF9tZW1iZXIocGVybWluZm8tPnVwZGF0ZWRDb2xzLAorCQkJCQkJCSAgIHJhbmdlQXR0bm8gLSBG
aXJzdExvd0ludmFsaWRIZWFwQXR0cmlidXRlTnVtYmVyKTsKKworCQkJTWVtb3J5Q29udGV4dFN3
aXRjaFRvKG9sZENvbnRleHQpOworCQl9CisJfQorCiAJLyogTWFwIHRoZSBjb2x1bW5zIHRvIGNo
aWxkJ3MgYXR0cmlidXRlIG51bWJlcnMgaWYgbmVlZGVkLiAqLwogCWlmIChyZWxpbmZvLT5yaV9S
b290UmVzdWx0UmVsSW5mbykKIAl7CkBAIC0xNDMwLDI5ICsxNDU1LDcgQEAgRXhlY0dldEV4dHJh
VXBkYXRlZENvbHMoUmVzdWx0UmVsSW5mbyAqcmVsaW5mbywgRVN0YXRlICplc3RhdGUpCiB7CiAJ
LyogQ29tcHV0ZSB0aGUgaW5mbyBpZiB3ZSBkaWRuJ3QgYWxyZWFkeSAqLwogCWlmICghcmVsaW5m
by0+cmlfZXh0cmFVcGRhdGVkQ29sc192YWxpZCkKLQl7Ci0JCWlmIChyZWxpbmZvLT5yaV9mb3JQ
b3J0aW9uT2YpCi0JCXsKLQkJCU1lbW9yeUNvbnRleHQgb2xkQ29udGV4dDsKLQotCQkJQXR0ck51
bWJlcglyYW5nZUF0dG5vID0gcmVsaW5mby0+cmlfZm9yUG9ydGlvbk9mLT5mcF9yYW5nZUF0dG5v
OwotCi0JCQlvbGRDb250ZXh0ID0gTWVtb3J5Q29udGV4dFN3aXRjaFRvKGVzdGF0ZS0+ZXNfcXVl
cnlfY3h0KTsKLQotCQkJLyoKLQkJCSAqIEZvciBVUERBVEUgLi4uIEZPUiBQT1JUSU9OIE9GLCB0
aGUgcmFuZ2UgY29sdW1uIGlzIGFjdHVhbGx5Ci0JCQkgKiBiZWluZyBtb2RpZmllZCAobmFycm93
ZWQgdmlhIGludGVyc2VjdGlvbiksIGJ1dCBpdCBpcyBub3QKLQkJCSAqIGluY2x1ZGVkIGluIHVw
ZGF0ZWRDb2xzIGJlY2F1c2UgdGhlIHVzZXIgZG9lcyBub3QgbmVlZCBVUERBVEUKLQkJCSAqIHBl
cm1pc3Npb24gb24gaXQuIFNvIHdlIG5lZWQgdG8gYWRkIGl0IHRvIHJpX2V4dHJhVXBkYXRlZENv
bHMKLQkJCSAqLwotCQkJcmVsaW5mby0+cmlfZXh0cmFVcGRhdGVkQ29scyA9Ci0JCQkJYm1zX2Fk
ZF9tZW1iZXIocmVsaW5mby0+cmlfZXh0cmFVcGRhdGVkQ29scywgcmFuZ2VBdHRubyAtIEZpcnN0
TG93SW52YWxpZEhlYXBBdHRyaWJ1dGVOdW1iZXIpOwotCi0JCQlNZW1vcnlDb250ZXh0U3dpdGNo
VG8ob2xkQ29udGV4dCk7Ci0JCX0KLQogCQlFeGVjSW5pdEdlbmVyYXRlZChyZWxpbmZvLCBlc3Rh
dGUsIENNRF9VUERBVEUpOwotCX0KIAlyZXR1cm4gcmVsaW5mby0+cmlfZXh0cmFVcGRhdGVkQ29s
czsKIH0KIApkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQvZXhlY3V0b3Ivbm9kZU1vZGlmeVRhYmxl
LmMgYi9zcmMvYmFja2VuZC9leGVjdXRvci9ub2RlTW9kaWZ5VGFibGUuYwppbmRleCA0Mzk1MDg5
NTY1Zi4uN2FmMDI2ODY0NmIgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL2V4ZWN1dG9yL25vZGVN
b2RpZnlUYWJsZS5jCisrKyBiL3NyYy9iYWNrZW5kL2V4ZWN1dG9yL25vZGVNb2RpZnlUYWJsZS5j
CkBAIC00NzcsMjEgKzQ3Nyw2IEBAIEV4ZWNJbml0R2VuZXJhdGVkKFJlc3VsdFJlbEluZm8gKnJl
c3VsdFJlbEluZm8sCiAJZWxzZQogCQl1cGRhdGVkQ29scyA9IE5VTEw7CiAKLQkvKgotCSAqIEZv
ciBVUERBVEUgLi4uIEZPUiBQT1JUSU9OIE9GLCB0aGUgcmFuZ2UgY29sdW1uIGlzIGFsc28gYmVp
bmcgbW9kaWZpZWQKLQkgKiAobmFycm93ZWQgdmlhIGludGVyc2VjdGlvbiksIGJ1dCBpdCBpcyBu
b3QgaW5jbHVkZWQgaW4gdXBkYXRlZENvbHMKLQkgKiBiZWNhdXNlIHRoZSB1c2VyIGRvZXMgbm90
IG5lZWQgVVBEQVRFIHBlcm1pc3Npb24gb24gaXQuICBXZSBtdXN0Ci0JICogYWNjb3VudCBmb3Ig
aXQgaGVyZSBzbyB0aGF0IGdlbmVyYXRlZCBjb2x1bW5zIHJlZmVyZW5jaW5nIHRoZSByYW5nZQot
CSAqIGNvbHVtbiBhcmUgcmVjb21wdXRlZC4KLQkgKi8KLQlpZiAocmVzdWx0UmVsSW5mby0+cmlf
Zm9yUG9ydGlvbk9mKQotCXsKLQkJQXR0ck51bWJlcglyYW5nZUF0dG5vID0gcmVzdWx0UmVsSW5m
by0+cmlfZm9yUG9ydGlvbk9mLT5mcF9yYW5nZUF0dG5vOwotCi0JCXVwZGF0ZWRDb2xzID0gYm1z
X2FkZF9tZW1iZXIoYm1zX2NvcHkodXBkYXRlZENvbHMpLAotCQkJCQkJCQkJIHJhbmdlQXR0bm8g
LSBGaXJzdExvd0ludmFsaWRIZWFwQXR0cmlidXRlTnVtYmVyKTsKLQl9Ci0KIAkvKgogCSAqIE1h
a2Ugc3VyZSB0aGVzZSBkYXRhIHN0cnVjdHVyZXMgYXJlIGJ1aWx0IGluIHRoZSBwZXItcXVlcnkg
bWVtb3J5CiAJICogY29udGV4dCBzbyB0aGV5J2xsIHN1cnZpdmUgdGhyb3VnaG91dCB0aGUgcXVl
cnkuCi0tIAoyLjM0LjEKCg==
--0000000000004f9560064fa38891--