Hi,
I want to suggest a patch against master (it may also be worth backporting
it) that makes it possible to use longer filenames (such as those with
absolute paths) in `BackgroundWorker.bgw_library_name`.
`BackgroundWorker.bgw_library_name` currently allows names up to
BGW_MAXLEN-1, which is gen
On Wed, Apr 26, 2023 at 03:07:18PM +0300, Aleksander Alekseev wrote:
> The commit message may require a bit of tweaking by the committer but
> other than that the patch seems to be fine. I'm going to mark it as
> RfC in a bit unless anyone objects.
In v4, I've introduced a new BGW_LIBLEN macro and
Hi Nathan,
On Fri, Jun 30, 2023 at 2:39 PM Nathan Bossart
wrote:
>
> In v4, I've introduced a new BGW_LIBLEN macro and set it to the default
> value of MAXPGPATH (1024). This way, the value can live in bgworker.h like
> the other BGW_* macros do. Plus, this should make the assertion that
> che
On Sun, Jul 02, 2023 at 04:37:52PM -0700, Yurii Rashkovskii wrote:
> Thank you for revising the patch. While this is relatively minor, I think
> it should be set to MAXPGPATH directly to clarify their relationship.
Committed. I set the size to MAXPGPATH directly instead of inventing a new
macro w
Nathan,
On Mon, Jul 3, 2023 at 3:08 PM Nathan Bossart
wrote:
> On Sun, Jul 02, 2023 at 04:37:52PM -0700, Yurii Rashkovskii wrote:
> > Thank you for revising the patch. While this is relatively minor, I think
> > it should be set to MAXPGPATH directly to clarify their relationship.
>
> Committed.
On Mon, Jul 03, 2023 at 06:00:12PM -0700, Yurii Rashkovskii wrote:
> Great, thank you! The reason I was leaving the other constant in place to
> make upgrading extensions trivial (so that they don't need to adjust for
> this), but if you think this is a better way, I am fine with it.
Sorry, I'm no
Nathan,
On Mon, Jul 3, 2023 at 8:12 PM Nathan Bossart
wrote:
> On Mon, Jul 03, 2023 at 06:00:12PM -0700, Yurii Rashkovskii wrote:
> > Great, thank you! The reason I was leaving the other constant in place to
> > make upgrading extensions trivial (so that they don't need to adjust for
> > this),
On Wed, Mar 15, 2023 at 10:38:34AM +0100, Daniel Gustafsson wrote:
> While here, I wonder if we should document what BGW_MAXLEN is defined as in
> bgworker.sgml?
I am -0.5 for this. If you are writing a new background worker, it's
probably reasonable to expect that you can locate the definition o
> On 21 Apr 2023, at 01:32, Nathan Bossart wrote:
>
> On Wed, Mar 15, 2023 at 10:38:34AM +0100, Daniel Gustafsson wrote:
>> While here, I wonder if we should document what BGW_MAXLEN is defined as in
>> bgworker.sgml?
>
> I am -0.5 for this. If you are writing a new background worker, it's
> pr
On Fri, Apr 21, 2023 at 10:49:48AM +0200, Daniel Gustafsson wrote:
> On 21 Apr 2023, at 01:32, Nathan Bossart wrote:
>> I am -0.5 for this. If you are writing a new background worker, it's
>> probably reasonable to expect that you can locate the definition of
>> BGW_MAXLEN.
>
> Of course. The
Hi,
> The trade-off of this patch is that the `BackgroundWorker` structure becomes
> larger. From my perspective, this is a reasonable cost (less than a kilobyte
> of extra space per worker).
Agree.
> The patch is backwards-compatible and ensures that bgw_library_name stays *at
> least* as lo
Aleksander,
On Mon, Apr 24, 2023 at 1:01 PM Aleksander Alekseev <
aleksan...@timescale.com> wrote:
> > The patch is backwards-compatible and ensures that bgw_library_name
> stays *at least* as long as BGW_MAXLEN. Existing external code that uses
> BGW_MAXLEN is a length boundary (for example, in
Hi,
> This is a very good catch and a suggestion. I've slightly reworked the patch,
> and I also made this static assertion to have less indirection and,
> therefore, make it easier to understand the premise.
> Version 2 is attached.
```
sizeof((BackgroundWorker *)NULL)->bgw_library_name
```
I
You're absolutely right. Here's v3.
On Mon, Apr 24, 2023 at 6:30 PM Aleksander Alekseev <
aleksan...@timescale.com> wrote:
> Hi,
>
> > This is a very good catch and a suggestion. I've slightly reworked the
> patch, and I also made this static assertion to have less indirection and,
> therefore,
Hi,
> You're absolutely right. Here's v3.
Please avoid using top posting [1].
The commit message may require a bit of tweaking by the committer but
other than that the patch seems to be fine. I'm going to mark it as
RfC in a bit unless anyone objects.
[1]: https://wiki.postgresql.org/wiki/Maili
On Mon, Mar 13, 2023 at 07:57:47AM -0700, Yurii Rashkovskii wrote:
> However, there are use cases where [potentially] longer names are
> expected/desired; for example, test benches (where library files may not
> [or can not] be copied to Postgres installation) or alternative library
> installation
Nathan,
Thank you for your review.
Indeed, my motivation for doing the change the way I did it was that only
bgw_library_name is expected to be longer, whereas it is much less of a
concern for other fields. If we have increased BGW_MAXLEN, it would have
increased the size of BackgroundWorker for
> On 13 Mar 2023, at 18:35, Nathan Bossart wrote:
>
> On Mon, Mar 13, 2023 at 07:57:47AM -0700, Yurii Rashkovskii wrote:
>> However, there are use cases where [potentially] longer names are
>> expected/desired; for example, test benches (where library files may not
>> [or can not] be copied to Po
18 matches
Mail list logo
