On Mon, Jul 29, 2019 at 10:55:29PM +0530, Jeevan Ladhe wrote: > I am attaching a patch that makes sure that *have_error is set to false in > pg_lsn_in_internal() itself, rather than being caller dependent.
Agreed about making the code more defensive as you do. I would keep the initialization in check_recovery_target_lsn and pg_lsn_in_internal though. That does not hurt and makes the code easier to understand, aka we don't expect an error by default in those paths. > IIUC, in the comment above we clearly want to mark 0 as an invalid lsn (also > further IIUC the comment states - lsn would start from (walSegSize + 1)). > Given this, should not it be invalid to allow "0/0" as the value of > type pg_lsn, or for that matter any number < walSegSize? You can rely on "0/0" as a base point to calculate the offset in a segment, so my guess is that we could break applications by generating an error. Please note that the behavior is much older than the introduction of pg_lsn, as the original parsing logic has been removed in 6f289c2b with validate_xlog_location() in xlogfuncs.c. -- Michael
signature.asc
Description: PGP signature