Content-Type: multipart/mixed; boundary="------------RK80fSOUORAuWRlrBnQvi52T"
Message-ID: <850c1b50-cc68-4b8b-8977-64da432f6644@pgbackrest.org>
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Improve checks for GUC recovery_target_xid
To: =?UTF-8?Q?H=C3=BCseyin_Demir?= <huseyin.d3r@gmail.com>,
 pgsql-hackers@lists.postgresql.org, Fujii Masao <masao.fujii@gmail.com>
References: <f14463ab-990b-4ae9-a177-998d2677aae0@pgbackrest.org>
 <f14463ab-990b-4ae9-a177-998d2677aae0@pgbackrest.org>
 <177209041824.626.6221932041384217360.pgcf@coridan.postgresql.org>
Content-Language: en-US
From: David Steele <david@pgbackrest.org>
In-Reply-To: 
 <177209041824.626.6221932041384217360.pgcf@coridan.postgresql.org>
Date: Wed,  4 Mar 2026 05:07:22 +0000 (UTC)
Archived-At: 
 <https://www.postgresql.org/message-id/850c1b50-cc68-4b8b-8977-64da432f6644%40pgbackrest.org>
Precedence: bulk

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

Thanks for having a look at this!

On 2/26/26 14:20, Hüseyin Demir wrote:
> 
> The following grammar can be changed by adding "without epoch must be greater than or equal to %u"
> +           GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.",
> +                               "recovery_target_xid",
> +                               FirstNormalTransactionId);

Oops - fixed!

> The comment on the lower-bound XID test says # Timeline target out of min range — should be # XID target out of min range.

I have fixed this and made the comments more consistent overall.

> When it comes to *endp validations I suppose the validation passes when we provide   recovery_target_xid = '-1'.  This passes the endp validation and FirstNormalTransactionId checks. Is it a valid approach to allow negative values to this GUC ?
> 
> When -1 is provided the following checks allow them to be a valid GUC.

Yeah, -1 should not be allowed here. I've updated the code to error on 
negative numbers but probably we should import strtou64_strict from the 
front end code or use strtou32_strict, though that needs to be discussed 
separately.

Thanks,
-David
--------------RK80fSOUORAuWRlrBnQvi52T
Content-Type: text/plain; charset=UTF-8; name="recovery-target-xid-v2.patch"
Content-Disposition: attachment; filename="recovery-target-xid-v2.patch"
Content-Transfer-Encoding: base64

RnJvbSA0ODgyNjBjMzBkZDY1YWFlMmYwYjkwNzc2NDFkYmU3ZTE5NWVhOGY0IE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBEYXZpZCBTdGVlbGUgPGRhdmlkQHBnbWFzdGVycy5u
ZXQ+CkRhdGU6IFdlZCwgNCBNYXIgMjAyNiAwNDo0NzoyMyArMDAwMApTdWJqZWN0OiBJbXBy
b3ZlIGNoZWNrcyBmb3IgR1VDIHJlY292ZXJ5X3RhcmdldF94aWQKCkN1cnJlbnRseSBjaGVj
a19yZWNvdmVyeV90YXJnZXRfeGlkKCkgY29udmVydHMgaW52YWxpZCB2YWx1ZXMgdG8gMC4g
U28sCmZvciBleGFtcGxlLCB0aGUgZm9sbG93aW5nIGNvbmZpZ3VyYXRpb24gYWRkZWQgdG8g
cG9zdGdyZXNxbC5jb25mCmZvbGxvd2VkIGJ5IGEgc3RhcnR1cDoKCnJlY292ZXJ5X3Rhcmdl
dF94aWQgPSAnYm9ndXMnCnJlY292ZXJ5X3RhcmdldF94aWQgPSAnMS4xJwpyZWNvdmVyeV90
YXJnZXRfeGlkID0gJzAnCgouLi4gZG9lcyBub3QgZ2VuZXJhdGUgYW4gZXJyb3IgYnV0IHJl
Y292ZXJ5IGRvZXMgbm90IGNvbXBsZXRlLiBUaGVyZSBhcmUKbWFueSB2YWx1ZXMgdGhhdCBj
YW4gcHJldmVudCByZWNvdmVyeSBmcm9tIGNvbXBsZXRpbmcgYnV0IHdlIHNob3VsZCBhdAps
ZWFzdCBjYXRjaCBvYnZpb3VzIG1pc2NvbmZpZ3VyYXRpb24gYnkgdGhlIHVzZXIuCgpUaGUg
b3JpZ2luIG9mIHRoZSBwcm9ibGVtIGlzIHRoYXQgd2UgZG8gbm90IHBlcmZvcm0gYSByYW5n
ZSBjaGVjayBpbiB0aGUKR1VDIHZhbHVlIHBhc3NlZC1pbiBmb3IgcmVjb3ZlcnlfdGFyZ2V0
X3hpZC4gVGhpcyBjb21taXQgaW1wcm92ZXMgdGhlCnNpdHVhdGlvbiBieSB1c2luZyBhZGRp
bmcgZW5kIGNoZWNraW5nIHRvIHN0cnRvdTY0KCkgYW5kIGJ5IHByb3ZpZGluZwpzdHJpY3Rl
ciByYW5nZSBjaGVja3MuIFNvbWUgdGVzdCBjYXNlcyBhcmUgYWRkZWQgZm9yIHRoZSBjYXNl
cyBvZiBhbgppbmNvcnJlY3Qgb3IgYSBsb3dlci1ib3VuZCB0aW1lbGluZSB2YWx1ZSwgY2hl
Y2tpbmcgdGhlIHNhbml0eSBvZiB0aGUKcmVwb3J0cyBiYXNlZCBvbiB0aGUgY29udGVudHMg
b2YgdGhlIHNlcnZlciBsb2dzLgoKQWxzbyBhZGQgYSBjb21tZW50IHRoYXQgdHJ1bmNhdGlv
biBvZiB0aGUgaW5wdXQgdmFsdWUgaXMgZXhwZWN0ZWQgc2luY2UKdXNlcnMgd2lsbCBnZW5l
cmFsbHkgYmUgdXNpbmcgdGhlIG91dHB1dCBmcm9tIHBnX2N1cnJlbnRfeGFjdF9pZCgpIChv
cgpzaW1pbGFyKSB0byBzZXQgcmVjb3ZlcnlfdGFyZ2V0X3hpZCAoanVzdCBhcyBvdXIgdGVz
dHMgZG8pLgotLS0KIHNyYy9iYWNrZW5kL2FjY2Vzcy90cmFuc2FtL3hsb2dyZWNvdmVyeS5j
ICAgfCAzMSArKysrKysrKysrKysrKysrKy0tCiBzcmMvdGVzdC9yZWNvdmVyeS90LzAwM19y
ZWNvdmVyeV90YXJnZXRzLnBsIHwgMzQgKysrKysrKysrKysrKysrKysrKysrCiAyIGZpbGVz
IGNoYW5nZWQsIDYzIGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0
IGEvc3JjL2JhY2tlbmQvYWNjZXNzL3RyYW5zYW0veGxvZ3JlY292ZXJ5LmMgYi9zcmMvYmFj
a2VuZC9hY2Nlc3MvdHJhbnNhbS94bG9ncmVjb3ZlcnkuYwppbmRleCBlY2Q2NmZkODZhNC4u
ZmQwMzQ1YTY1ZjggMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL2FjY2Vzcy90cmFuc2FtL3hs
b2dyZWNvdmVyeS5jCisrKyBiL3NyYy9iYWNrZW5kL2FjY2Vzcy90cmFuc2FtL3hsb2dyZWNv
dmVyeS5jCkBAIC01MTA4LDExICs1MTA4LDM4IEBAIGNoZWNrX3JlY292ZXJ5X3RhcmdldF94
aWQoY2hhciAqKm5ld3ZhbCwgdm9pZCAqKmV4dHJhLCBHdWNTb3VyY2Ugc291cmNlKQogCXsK
IAkJVHJhbnNhY3Rpb25JZCB4aWQ7CiAJCVRyYW5zYWN0aW9uSWQgKm15ZXh0cmE7CisJCWNo
YXIJCSAgKmVuZHA7CisJCWNoYXIgCQkgICp2YWw7CiAKIAkJZXJybm8gPSAwOwotCQl4aWQg
PSAoVHJhbnNhY3Rpb25JZCkgc3RydG91NjQoKm5ld3ZhbCwgTlVMTCwgMCk7Ci0JCWlmIChl
cnJubyA9PSBFSU5WQUwgfHwgZXJybm8gPT0gRVJBTkdFKQorCisJCS8qCisJCSAqIENvbnN1
bWUgbGVhZGluZyB3aGl0ZXNwYWNlIHRvIGRldGVybWluZSBpZiBudW1iZXIgaXMgbmVnYXRp
dmUKKwkJICovCisJCXZhbCA9ICpuZXd2YWw7CisKKwkJd2hpbGUgKGlzc3BhY2UoKHVuc2ln
bmVkIGNoYXIpKnZhbCkpCisJCQl2YWwrKzsKKworCQkvKgorCQkgKiBUaGlzIGNhc3Qgd2ls
bCByZW1vdmUgdGhlIGVwb2NoLCBpZiBhbnkKKwkJICovCisJCXhpZCA9IChUcmFuc2FjdGlv
bklkKSBzdHJ0b3U2NCh2YWwsICZlbmRwLCAwKTsKKworCQlpZiAoKmVuZHAgIT0gJ1wwJyB8
fCBlcnJubyA9PSBFSU5WQUwgfHwgZXJybm8gPT0gRVJBTkdFIHx8ICp2YWwgPT0gJy0nKQor
CQl7CisJCQlHVUNfY2hlY2tfZXJyZGV0YWlsKCJcIiVzXCIgaXMgbm90IGEgdmFsaWQgbnVt
YmVyLiIsCisJCQkJCQkJCSJyZWNvdmVyeV90YXJnZXRfeGlkIik7CiAJCQlyZXR1cm4gZmFs
c2U7CisJCX0KKworCQlpZiAoeGlkIDwgRmlyc3ROb3JtYWxUcmFuc2FjdGlvbklkKQorCQl7
CisJCQlHVUNfY2hlY2tfZXJyZGV0YWlsKCJcIiVzXCIgd2l0aG91dCBlcG9jaCBtdXN0IGJl
IGdyZWF0ZXIgdGhhbiBvciBlcXVhbCB0byAldS4iLAorCQkJCQkJCQkicmVjb3ZlcnlfdGFy
Z2V0X3hpZCIsCisJCQkJCQkJCUZpcnN0Tm9ybWFsVHJhbnNhY3Rpb25JZCk7CisJCQlyZXR1
cm4gZmFsc2U7CisJCX0KIAogCQlteWV4dHJhID0gKFRyYW5zYWN0aW9uSWQgKikgZ3VjX21h
bGxvYyhMT0csIHNpemVvZihUcmFuc2FjdGlvbklkKSk7CiAJCWlmICghbXlleHRyYSkKZGlm
ZiAtLWdpdCBhL3NyYy90ZXN0L3JlY292ZXJ5L3QvMDAzX3JlY292ZXJ5X3RhcmdldHMucGwg
Yi9zcmMvdGVzdC9yZWNvdmVyeS90LzAwM19yZWNvdmVyeV90YXJnZXRzLnBsCmluZGV4IGUw
ZGYxYTIzNDIzLi4wODEzMzk0ZDcwYSAxMDA2NDQKLS0tIGEvc3JjL3Rlc3QvcmVjb3Zlcnkv
dC8wMDNfcmVjb3ZlcnlfdGFyZ2V0cy5wbAorKysgYi9zcmMvdGVzdC9yZWNvdmVyeS90LzAw
M19yZWNvdmVyeV90YXJnZXRzLnBsCkBAIC0yNDAsNCArMjQwLDM4IEBAIG9rKCEkcmVzLCAn
aW52YWxpZCB0aW1lbGluZSB0YXJnZXQgKHVwcGVyIGJvdW5kIGNoZWNrKScpOwogJGxvZ19z
dGFydCA9CiAgICRub2RlX3N0YW5kYnktPndhaXRfZm9yX2xvZygibXVzdCBiZSBiZXR3ZWVu
IDEgYW5kIDQyOTQ5NjcyOTUiLCAkbG9nX3N0YXJ0KTsKIAorIyBJbnZhbGlkIHJlY292ZXJ5
X3RhcmdldF94aWQgKGJvZ3VzIHZhbHVlKQorJG5vZGVfc3RhbmRieSA9IFBvc3RncmVTUUw6
OlRlc3Q6OkNsdXN0ZXItPm5ldygnc3RhbmRieV8xMCcpOworJG5vZGVfc3RhbmRieS0+aW5p
dF9mcm9tX2JhY2t1cCgkbm9kZV9wcmltYXJ5LCAnbXlfYmFja3VwJywKKwloYXNfcmVzdG9y
aW5nID0+IDEpOworJG5vZGVfc3RhbmRieS0+YXBwZW5kX2NvbmYoJ3Bvc3RncmVzcWwuY29u
ZicsCisJInJlY292ZXJ5X3RhcmdldF94aWQgPSAnYm9ndXMnIik7CisKKyRyZXMgPSBydW5f
bG9nKAorCVsKKwkJJ3BnX2N0bCcsCisJCSctLXBnZGF0YScgPT4gJG5vZGVfc3RhbmRieS0+
ZGF0YV9kaXIsCisJCSctLWxvZycgPT4gJG5vZGVfc3RhbmRieS0+bG9nZmlsZSwKKwkJJ3N0
YXJ0JywKKwldKTsKK29rKCEkcmVzLCAnaW52YWxpZCByZWNvdmVyeV90YXJnZXRfeGlkIChi
b2d1cyB2YWx1ZSknKTsKKworJGxvZ19zdGFydCA9ICRub2RlX3N0YW5kYnktPndhaXRfZm9y
X2xvZygiaXMgbm90IGEgdmFsaWQgbnVtYmVyIik7CisKKyMgSW52YWxpZCByZWNvdmVyeV90
YXJnZXRfeGlkIChsb3dlciBib3VuZCBjaGVjaykKKyRub2RlX3N0YW5kYnktPmFwcGVuZF9j
b25mKCdwb3N0Z3Jlc3FsLmNvbmYnLAorCSJyZWNvdmVyeV90YXJnZXRfeGlkID0gJzAnIik7
CisKKyRyZXMgPSBydW5fbG9nKAorCVsKKwkJJ3BnX2N0bCcsCisJCSctLXBnZGF0YScgPT4g
JG5vZGVfc3RhbmRieS0+ZGF0YV9kaXIsCisJCSctLWxvZycgPT4gJG5vZGVfc3RhbmRieS0+
bG9nZmlsZSwKKwkJJ3N0YXJ0JywKKwldKTsKK29rKCEkcmVzLCAnaW52YWxpZCByZWNvdmVy
eV90YXJnZXRfeGlkIChsb3dlciBib3VuZCBjaGVjayknKTsKKworJGxvZ19zdGFydCA9ICRu
b2RlX3N0YW5kYnktPndhaXRfZm9yX2xvZygKKwkid2l0aG91dCBlcG9jaCBtdXN0IGJlIGdy
ZWF0ZXIgdGhhbiBvciBlcXVhbCB0byAzIiwgJGxvZ19zdGFydCk7CisKIGRvbmVfdGVzdGlu
ZygpOwotLSAKMi4zNC4xCgo=

--------------RK80fSOUORAuWRlrBnQvi52T--