At Mon, 27 Jan 2020 16:55:56 +0900, Michael Paquier <mich...@paquier.xyz> wrote in > On Sun, Jan 26, 2020 at 06:47:57PM -0800, Mark Dilger wrote: > > There is something unusual about comparing a XLogSegNo variable in > > this way, but it seems to go back to 2014 when the replication slots > > were introduced in commit 858ec11858a914d4c380971985709b6d6b7dd6fc, > > and XLogSegNo was unsigned then, too. Depending on how you look at > > it, this could be a thinko, or it could be defensive programming > > against future changes to the XLogSegNo typedef. I’m betting it was > > defensive programming, given the context. As such, I don’t think it > > would be appropriate to remove this defense in your patch. > > Yeah. To e honest, I am not actually sure if it is worth bothering > about any of those three places.
+1. FWIW, I have reasons for being aganst the first the the last items. For the first item, The duplicate if blocks seem working as enclosure of a meaningful set of code. It's annoying that OwnLatch follows a bunch of "else if() ereport" lines in a block. For the last item, using '==' in the context of size comparison make me a bit uneasy. I prefer '< 1' there but I don't bother doing that. They are logially the same. For the second item, I don't object to do that but also I'm not willing to support it. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
