| From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
|---|---|
| To: | Aleksander Alekseev <aleksander(at)timescale(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz> |
| Subject: | Re: [PATCH] Slight improvement of worker_spi.c example |
| Date: | 2023-06-13 11:58:02 |
| Message-ID: | 20230613115802.wz5ifb4mkyiu5exo@jrouhaud |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | 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.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David Steele | 2023-06-13 12:40:12 | Re: Views no longer in rangeTabls? |
| Previous Message | James Pang (chaolpan) | 2023-06-13 11:32:54 | RE: extended statistics n-distinct on multiple columns not used when join two tables |