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.
