Content-Type: multipart/mixed; boundary="------------oybFGwUgb6p0sfBZU7WwpSjA"
Message-ID: <c2e97e35-1227-4351-8faa-fa95208f6cf9@pgbackrest.org>
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Improve checks for GUC recovery_target_xid
To: Fujii Masao <masao.fujii@gmail.com>
Cc: =?UTF-8?Q?H=C3=BCseyin_Demir?= <huseyin.d3r@gmail.com>,
 Pg Hackers <pgsql-hackers@postgresql.org>
References: <f14463ab-990b-4ae9-a177-998d2677aae0@pgbackrest.org>
 <CAHGQGwEnakTrosMp3Y=Trya4MTC-h8Lqx3FDU1mOALRQ0rz59Q@mail.gmail.com>
 <9e8b7ee9-4a16-477a-baa5-0cdf37a04798@pgbackrest.org>
 <CAHGQGwG44vZbSoBmg076G+xkR6n=Tj2=q+fVkfP7yEsyF1daFA@mail.gmail.com>
Content-Language: en-US
From: David Steele <david@pgbackrest.org>
In-Reply-To: 
 <CAHGQGwG44vZbSoBmg076G+xkR6n=Tj2=q+fVkfP7yEsyF1daFA@mail.gmail.com>
Date: Thu,  5 Mar 2026 03:40:44 +0000 (UTC)
Archived-At: 
 <https://www.postgresql.org/message-id/c2e97e35-1227-4351-8faa-fa95208f6cf9%40pgbackrest.org>
Precedence: bulk

This is a multi-part message in MIME format.
--------------oybFGwUgb6p0sfBZU7WwpSjA
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 3/4/26 22:41, Fujii Masao wrote:
> 
> Regarding the regression test, if the purpose is to verify the GUC hook
> for recovery_target_xid, it might be simpler to test whether
> "ALTER SYSTEM SET recovery_target_xid TO ..." succeeds or fails as expected
> as follows, rather than starting the server with that setting. That said,
> since recovery_target_timeline is already tested in a similar way, I understand
> why you followed the same pattern here. So I'm ok with the current approach.

I wrote the tests for recovery_target_timeline but I was not too 
satisfied with them because starting Postgres is fairly expensive.

> 
>      my ($result, $stdout, $stderr) = $node_primary->psql('postgres',
>          "ALTER SYSTEM SET recovery_target_xid TO 'bogus'");
>      like(
>          $stderr,
>          qr/is not a valid number/,
>          "invalid recovery_target_xid (bogus value)");
> 
> If we think it's better to use ALTER SYSTEM SET for testing invalid
> recovery_target_xxx settings to keep the regression tests simpler,
> we can revisit this later and address it in a separate patch.

I've updated the patch to do it this way. Not only is it faster but you 
get a better message when the expected value is incorrect.

I can update the tests for recovery_target_timeline in a separate patch.

Regards,
-David

--------------oybFGwUgb6p0sfBZU7WwpSjA
Content-Type: text/plain; charset=UTF-8; name="recovery-target-xid-v3.patch"
Content-Disposition: attachment; filename="recovery-target-xid-v3.patch"
Content-Transfer-Encoding: base64

RnJvbSBiMmJlNTlhNTJhN2E3NGNkMGE2YzgzNjE5ZmE3ODJhOTJiN2FiOWEwIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBEYXZpZCBTdGVlbGUgPGRhdmlkQHBnbWFzdGVycy5u
ZXQ+CkRhdGU6IFRodSwgNSBNYXIgMjAyNiAwMzozNTozMiArMDAwMApTdWJqZWN0OiBJbXBy
b3ZlIGNoZWNrcyBmb3IgR1VDIHJlY292ZXJ5X3RhcmdldF94aWQKCkN1cnJlbnRseSBjaGVj
a19yZWNvdmVyeV90YXJnZXRfeGlkKCkgY29udmVydHMgaW52YWxpZCB2YWx1ZXMgdG8gMC4g
U28sCmZvciBleGFtcGxlLCB0aGUgZm9sbG93aW5nIGNvbmZpZ3VyYXRpb24gYWRkZWQgdG8g
cG9zdGdyZXNxbC5jb25mCmZvbGxvd2VkIGJ5IGEgc3RhcnR1cDoKCnJlY292ZXJ5X3Rhcmdl
dF94aWQgPSAnYm9ndXMnCnJlY292ZXJ5X3RhcmdldF94aWQgPSAnMS4xJwpyZWNvdmVyeV90
YXJnZXRfeGlkID0gJzAnCgouLi4gZG9lcyBub3QgZ2VuZXJhdGUgYW4gZXJyb3IgYnV0IHJl
Y292ZXJ5IGRvZXMgbm90IGNvbXBsZXRlLiBUaGVyZSBhcmUKbWFueSB2YWx1ZXMgdGhhdCBj
YW4gcHJldmVudCByZWNvdmVyeSBmcm9tIGNvbXBsZXRpbmcgYnV0IHdlIHNob3VsZCBhdAps
ZWFzdCBjYXRjaCBvYnZpb3VzIG1pc2NvbmZpZ3VyYXRpb24gYnkgdGhlIHVzZXIuCgpUaGUg
b3JpZ2luIG9mIHRoZSBwcm9ibGVtIGlzIHRoYXQgd2UgZG8gbm90IHBlcmZvcm0gYSByYW5n
ZSBjaGVjayBpbiB0aGUKR1VDIHZhbHVlIHBhc3NlZC1pbiBmb3IgcmVjb3ZlcnlfdGFyZ2V0
X3hpZC4gVGhpcyBjb21taXQgaW1wcm92ZXMgdGhlCnNpdHVhdGlvbiBieSB1c2luZyBhZGRp
bmcgZW5kIGNoZWNraW5nIHRvIHN0cnRvdTY0KCkgYW5kIGJ5IHByb3ZpZGluZwpzdHJpY3Rl
ciByYW5nZSBjaGVja3MuIFNvbWUgdGVzdCBjYXNlcyBhcmUgYWRkZWQgZm9yIHRoZSBjYXNl
cyBvZiBhbgppbmNvcnJlY3Qgb3IgYSBsb3dlci1ib3VuZCB0aW1lbGluZSB2YWx1ZSwgY2hl
Y2tpbmcgdGhlIHNhbml0eSBvZiB0aGUKcmVwb3J0cyBiYXNlZCBvbiB0aGUgY29udGVudHMg
b2YgdGhlIHNlcnZlciBsb2dzLgoKQWRkIGEgY29tbWVudCB0aGF0IHRydW5jYXRpb24gb2Yg
dGhlIGlucHV0IHZhbHVlIGlzIGV4cGVjdGVkIHNpbmNlIHVzZXJzCndpbGwgZ2VuZXJhbGx5
IGJlIHVzaW5nIHRoZSBvdXRwdXQgZnJvbSBwZ19jdXJyZW50X3hhY3RfaWQoKSAob3IKc2lt
aWxhcikgdG8gc2V0IHJlY292ZXJ5X3RhcmdldF94aWQgKGp1c3QgYXMgb3VyIHRlc3RzIGRv
KS4KCkFsc28gdXBkYXRlIHRoZSBkb2N1bWVudGF0aW9uIGZvciByZWNvdmVyeV90YXJnZXRf
eGlkIHRvIGNsYXJpZnkgdXNhZ2UuCi0tLQogZG9jL3NyYy9zZ21sL2NvbmZpZy5zZ21sICAg
ICAgICAgICAgICAgICAgICB8IDE1ICsrKysrKysrKysKIHNyYy9iYWNrZW5kL2FjY2Vzcy90
cmFuc2FtL3hsb2dyZWNvdmVyeS5jICAgfCAzMSArKysrKysrKysrKysrKysrKysrLS0KIHNy
Yy90ZXN0L3JlY292ZXJ5L3QvMDAzX3JlY292ZXJ5X3RhcmdldHMucGwgfCAyNyArKysrKysr
KysrKysrKysrKysKIDMgZmlsZXMgY2hhbmdlZCwgNzEgaW5zZXJ0aW9ucygrKSwgMiBkZWxl
dGlvbnMoLSkKCmRpZmYgLS1naXQgYS9kb2Mvc3JjL3NnbWwvY29uZmlnLnNnbWwgYi9kb2Mv
c3JjL3NnbWwvY29uZmlnLnNnbWwKaW5kZXggZjY3MGUyZDRjMzEuLjk0OWI4NmU0ZTcwIDEw
MDY0NAotLS0gYS9kb2Mvc3JjL3NnbWwvY29uZmlnLnNnbWwKKysrIGIvZG9jL3NyYy9zZ21s
L2NvbmZpZy5zZ21sCkBAIC00MzM0LDYgKzQzMzQsMjEgQEAgcmVzdG9yZV9jb21tYW5kID0g
J2NvcHkgIkM6XFxzZXJ2ZXJcXGFyY2hpdmVkaXJcXCVmIiAiJXAiJyAgIyBXaW5kb3dzCiAg
ICAgICAgIFRoZSBwcmVjaXNlIHN0b3BwaW5nIHBvaW50IGlzIGFsc28gaW5mbHVlbmNlZCBi
eQogICAgICAgICA8eHJlZiBsaW5rZW5kPSJndWMtcmVjb3ZlcnktdGFyZ2V0LWluY2x1c2l2
ZSIvPi4KICAgICAgICA8L3BhcmE+CisKKyAgICAgICA8cGFyYT4KKyAgICAgICAgVGhlIHZh
bHVlIGNhbiBiZSBzcGVjaWZpZWQgYXMgZWl0aGVyIGEgMzItYml0IHRyYW5zYWN0aW9uIElE
IG9yIGEgNjQtYml0CisgICAgICAgIHRyYW5zYWN0aW9uIElEIChjb25zaXN0aW5nIG9mIGFu
IGVwb2NoIGFuZCBhIDMyLWJpdCBJRCksIHN1Y2ggYXMgdGhlCisgICAgICAgIHZhbHVlIHJl
dHVybmVkIGJ5IDxmdW5jdGlvbj5wZ19jdXJyZW50X3hhY3RfaWQoKTwvZnVuY3Rpb24+LiBX
aGVuIGEKKyAgICAgICAgNjQtYml0IHRyYW5zYWN0aW9uIElEIGlzIHByb3ZpZGVkLCBvbmx5
IGl0cyAzMi1iaXQgdHJhbnNhY3Rpb24gSUQKKyAgICAgICAgcG9ydGlvbiBpcyB1c2VkIGFz
IHRoZSByZWNvdmVyeSB0YXJnZXQuIEZvciBleGFtcGxlLCB0aGUgdmFsdWVzCisgICAgICAg
IDQyOTQ5NjgyOTYgKGVwb2NoIDEpIGFuZCA4NTg5OTM1NTkyIChlcG9jaCAyKSBib3RoIHJl
ZmVyIHRvIHRoZSBzYW1lCisgICAgICAgIDMyLWJpdCB0cmFuc2FjdGlvbiBJRCwgMTAwMC4K
KyAgICAgICA8L3BhcmE+CisKKyAgICAgICA8cGFyYT4KKyAgICAgICAgVGhlIGVmZmVjdGl2
ZSB0cmFuc2FjdGlvbiBJRCAodGhlIDMyLWJpdCBwb3J0aW9uKSBtdXN0IGJlIGdyZWF0ZXIg
dGhhbgorICAgICAgICBvciBlcXVhbCB0byAzLgorICAgICAgIDwvcGFyYT4KICAgICAgIDwv
bGlzdGl0ZW0+CiAgICAgIDwvdmFybGlzdGVudHJ5PgogCmRpZmYgLS1naXQgYS9zcmMvYmFj
a2VuZC9hY2Nlc3MvdHJhbnNhbS94bG9ncmVjb3ZlcnkuYyBiL3NyYy9iYWNrZW5kL2FjY2Vz
cy90cmFuc2FtL3hsb2dyZWNvdmVyeS5jCmluZGV4IGVjZDY2ZmQ4NmE0Li5mZDAzNDVhNjVm
OCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvYWNjZXNzL3RyYW5zYW0veGxvZ3JlY292ZXJ5
LmMKKysrIGIvc3JjL2JhY2tlbmQvYWNjZXNzL3RyYW5zYW0veGxvZ3JlY292ZXJ5LmMKQEAg
LTUxMDgsMTEgKzUxMDgsMzggQEAgY2hlY2tfcmVjb3ZlcnlfdGFyZ2V0X3hpZChjaGFyICoq
bmV3dmFsLCB2b2lkICoqZXh0cmEsIEd1Y1NvdXJjZSBzb3VyY2UpCiAJewogCQlUcmFuc2Fj
dGlvbklkIHhpZDsKIAkJVHJhbnNhY3Rpb25JZCAqbXlleHRyYTsKKwkJY2hhcgkJICAqZW5k
cDsKKwkJY2hhciAJCSAgKnZhbDsKIAogCQllcnJubyA9IDA7Ci0JCXhpZCA9IChUcmFuc2Fj
dGlvbklkKSBzdHJ0b3U2NCgqbmV3dmFsLCBOVUxMLCAwKTsKLQkJaWYgKGVycm5vID09IEVJ
TlZBTCB8fCBlcnJubyA9PSBFUkFOR0UpCisKKwkJLyoKKwkJICogQ29uc3VtZSBsZWFkaW5n
IHdoaXRlc3BhY2UgdG8gZGV0ZXJtaW5lIGlmIG51bWJlciBpcyBuZWdhdGl2ZQorCQkgKi8K
KwkJdmFsID0gKm5ld3ZhbDsKKworCQl3aGlsZSAoaXNzcGFjZSgodW5zaWduZWQgY2hhcikq
dmFsKSkKKwkJCXZhbCsrOworCisJCS8qCisJCSAqIFRoaXMgY2FzdCB3aWxsIHJlbW92ZSB0
aGUgZXBvY2gsIGlmIGFueQorCQkgKi8KKwkJeGlkID0gKFRyYW5zYWN0aW9uSWQpIHN0cnRv
dTY0KHZhbCwgJmVuZHAsIDApOworCisJCWlmICgqZW5kcCAhPSAnXDAnIHx8IGVycm5vID09
IEVJTlZBTCB8fCBlcnJubyA9PSBFUkFOR0UgfHwgKnZhbCA9PSAnLScpCisJCXsKKwkJCUdV
Q19jaGVja19lcnJkZXRhaWwoIlwiJXNcIiBpcyBub3QgYSB2YWxpZCBudW1iZXIuIiwKKwkJ
CQkJCQkJInJlY292ZXJ5X3RhcmdldF94aWQiKTsKIAkJCXJldHVybiBmYWxzZTsKKwkJfQor
CisJCWlmICh4aWQgPCBGaXJzdE5vcm1hbFRyYW5zYWN0aW9uSWQpCisJCXsKKwkJCUdVQ19j
aGVja19lcnJkZXRhaWwoIlwiJXNcIiB3aXRob3V0IGVwb2NoIG11c3QgYmUgZ3JlYXRlciB0
aGFuIG9yIGVxdWFsIHRvICV1LiIsCisJCQkJCQkJCSJyZWNvdmVyeV90YXJnZXRfeGlkIiwK
KwkJCQkJCQkJRmlyc3ROb3JtYWxUcmFuc2FjdGlvbklkKTsKKwkJCXJldHVybiBmYWxzZTsK
KwkJfQogCiAJCW15ZXh0cmEgPSAoVHJhbnNhY3Rpb25JZCAqKSBndWNfbWFsbG9jKExPRywg
c2l6ZW9mKFRyYW5zYWN0aW9uSWQpKTsKIAkJaWYgKCFteWV4dHJhKQpkaWZmIC0tZ2l0IGEv
c3JjL3Rlc3QvcmVjb3ZlcnkvdC8wMDNfcmVjb3ZlcnlfdGFyZ2V0cy5wbCBiL3NyYy90ZXN0
L3JlY292ZXJ5L3QvMDAzX3JlY292ZXJ5X3RhcmdldHMucGwKaW5kZXggZTBkZjFhMjM0MjMu
LmY1ZGExMzEyOGQ2IDEwMDY0NAotLS0gYS9zcmMvdGVzdC9yZWNvdmVyeS90LzAwM19yZWNv
dmVyeV90YXJnZXRzLnBsCisrKyBiL3NyYy90ZXN0L3JlY292ZXJ5L3QvMDAzX3JlY292ZXJ5
X3RhcmdldHMucGwKQEAgLTI0MCw0ICsyNDAsMzEgQEAgb2soISRyZXMsICdpbnZhbGlkIHRp
bWVsaW5lIHRhcmdldCAodXBwZXIgYm91bmQgY2hlY2spJyk7CiAkbG9nX3N0YXJ0ID0KICAg
JG5vZGVfc3RhbmRieS0+d2FpdF9mb3JfbG9nKCJtdXN0IGJlIGJldHdlZW4gMSBhbmQgNDI5
NDk2NzI5NSIsICRsb2dfc3RhcnQpOwogCisjIEludmFsaWQgcmVjb3ZlcnlfdGFyZ2V0X3hp
ZCB0ZXN0cworJG5vZGVfc3RhbmRieSA9IFBvc3RncmVTUUw6OlRlc3Q6OkNsdXN0ZXItPm5l
dygnc3RhbmRieV8xMCcpOworJG5vZGVfc3RhbmRieS0+aW5pdF9mcm9tX2JhY2t1cCgkbm9k
ZV9wcmltYXJ5LCAnbXlfYmFja3VwJywKKwloYXNfcmVzdG9yaW5nID0+IDEpOworJG5vZGVf
c3RhbmRieS0+c3RhcnQ7CisKK215ICgkcmVzdWx0LCAkc3Rkb3V0LCAkc3RkZXJyKSA9ICRu
b2RlX3ByaW1hcnktPnBzcWwoJ3Bvc3RncmVzJywKKwkiQUxURVIgU1lTVEVNIFNFVCByZWNv
dmVyeV90YXJnZXRfeGlkIFRPICdib2d1cyciKTsKK2xpa2UoCisJJHN0ZGVyciwKKwlxci9p
cyBub3QgYSB2YWxpZCBudW1iZXIvLAorCSJpbnZhbGlkIHJlY292ZXJ5X3RhcmdldF94aWQg
KGJvZ3VzIHZhbHVlKSIpOworCisoJHJlc3VsdCwgJHN0ZG91dCwgJHN0ZGVycikgPSAkbm9k
ZV9wcmltYXJ5LT5wc3FsKCdwb3N0Z3JlcycsCisJIkFMVEVSIFNZU1RFTSBTRVQgcmVjb3Zl
cnlfdGFyZ2V0X3hpZCBUTyAnLTEnIik7CitsaWtlKAorCSRzdGRlcnIsCisJcXIvaXMgbm90
IGEgdmFsaWQgbnVtYmVyLywKKwkiaW52YWxpZCByZWNvdmVyeV90YXJnZXRfeGlkIChuZWdh
dGl2ZSkiKTsKKworKCRyZXN1bHQsICRzdGRvdXQsICRzdGRlcnIpID0gJG5vZGVfcHJpbWFy
eS0+cHNxbCgncG9zdGdyZXMnLAorCSJBTFRFUiBTWVNURU0gU0VUIHJlY292ZXJ5X3Rhcmdl
dF94aWQgVE8gJzAnIik7CitsaWtlKAorCSRzdGRlcnIsCisJcXIvd2l0aG91dCBlcG9jaCBt
dXN0IGJlIGdyZWF0ZXIgdGhhbiBvciBlcXVhbCB0byAzLywKKwkiaW52YWxpZCByZWNvdmVy
eV90YXJnZXRfeGlkIChsb3dlciBib3VuZCBjaGVjaykiKTsKKwogZG9uZV90ZXN0aW5nKCk7
Ci0tIAoyLjM0LjEKCg==

--------------oybFGwUgb6p0sfBZU7WwpSjA--