Re: extension_control_path and "directory"

From: Matheus Alcantara <matheusssilv97(at)gmail(dot)com>
To: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
Cc: Christoph Berg <myon(at)debian(dot)org>, Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: extension_control_path and "directory"
Date: 2025-04-29 13:49:09
Message-ID: CAFY6G8cmNw6H=Xz8Q45eKVy9aSKLy+GASGiq55nsnyxVo_pcwg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Mon, Apr 28, 2025 at 5:49 PM David E. Wheeler <david(at)justatheory(dot)com> wrote:
>
> > To append the /extension at each path on extension_control_path would
> > require some changes on find_in_path() that
> > find_extension_control_filename() calls, which I think that it would
> > make the function more complicated. I've them created a similar
> > find_in_paths() function that works in the same way but it receives a
> > List of paths instead of the string of paths separated by ":". We can
> > get this List of paths using get_extension_control_directories() that
> > also handle the macro substitution like find_in_path().
> >
> > Attached v4 with these fixes. I hope that now you should be able to omit
> > the /extension from the GUC value.
>
> Yes! It now works with this configuration:
>
> ```ini
> extension_control_path = '/Users/david/Downloads/share/postgresql:$system'
> dynamic_library_path = '/Users/david/Downloads/lib/postgresql:$libdir’
> ```
>
> Which is nicely more consistent. Kind of want that first one to be called “share_path” now, though, since it’s not just extensions. Although I guess it’s only extension control file searching that uses it (for now).
>
Thanks for testing!

> If I understand this bit correctly:
>
> ```c
> /* Substitute the path macro if needed */
> mangled = substitute_path_macro(piece, "$system", system_dir);
>
> /*
> * Append "extension" suffix in case is a custom extension control
> * path.
> */
> if (strcmp(piece, "$system") != 0)
> mangled = psprintf("%s/extension", mangled);
> ```
>
> The value of `piece` is a single path from the search path, right? If so, I think it’s either `$system` or something else; I don’t it would ever be that `$system` is a substring of a single path. Is that right?
>
Yes, it is a single path from the search path, in your case it will be
"/Users/david/Downloads/share/postgresql" and "$system". We split these
paths based on the system path separator and get the next "piece" here:

char *piece = first_path_var_separator(ecp);

The first_path_var_separator() changes the "ecp" parameter on every call,
it returns the next path on "ecp" and changes it to have the remaining
paths to iterate over it.

> Other than that, I think this patch is good to go. Thanks!
>
Thanks for reviewing!

--
Matheus Alcantara

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Andres Freund 2025-04-29 13:52:03 Re: pgsql: doc: clarify new behavior of max_files_per_process
Previous Message Bruce Momjian 2025-04-29 13:45:44 pgsql: doc: clarify new behavior of max_files_per_process

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrei Lepikhov 2025-04-29 13:52:40 Re: MergeAppend could consider sorting cheapest child path
Previous Message Rahila Syed 2025-04-29 13:13:07 Re: Enhancing Memory Context Statistics Reporting