Re: [PATCH] Slight improvement of worker_spi.c example - Mailing list pgsql-hackers

From Julien Rouhaud
Subject Re: [PATCH] Slight improvement of worker_spi.c example
Date
Msg-id 20230613115802.wz5ifb4mkyiu5exo@jrouhaud
Whole thread Raw
In response to Re: [PATCH] Slight improvement of worker_spi.c example  (Aleksander Alekseev <aleksander@timescale.com>)
Responses Re: [PATCH] Slight improvement of worker_spi.c example
List pgsql-hackers
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.



pgsql-hackers by date:

Previous
From: "James Pang (chaolpan)"
Date:
Subject: RE: extended statistics n-distinct on multiple columns not used when join two tables
Next
From: David Steele
Date:
Subject: Re: Views no longer in rangeTabls?