Re: [PATCH] Slight improvement of worker_spi.c example

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: Raw Message | Whole Thread | 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.

In response to

Responses

Browse pgsql-hackers by date

  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