On Tue, Jun 13, 2023 at 12:34:09PM +0300, Aleksander Alekseev wrote:
>
> > I agree that the current code
> > could lead folks to think that PushActiveSnapshot must go after
> > SPI_connect, but wouldn't the reverse ordering just give folks the opposite
> > impression?
>
> This is the exact reason why the original patch had an explicit
> comment that the ordering is not important in this case. It was argued
> however that the comment is redundant and thus it was removed.
I also don't think that a comment is really worthwhile. If there were any hard
dependency, it should be mentioned in the various functions comments as that's
the first place one should look at when using a function they're not familiar
with.
That being said, I still don't understand why you focus on this tiny and not
really important detail while the module itself is actually broken (for dynamic
bgworker without s_p_l) and also has some broken behaviors with regards to the
naptime that are way more likely to hurt third party code that was written
using this module as an example.