Hi,
Thanks for reviewing and suggestions!
On Thu, Mar 6, 2025 at 10:46 AM Peter Eisentraut <peter@eisentraut.org> wrote:
> This looks very good to me. I have one issue to point out: The logic
> in get_extension_control_directories() needs to be a little bit more
> careful to align with the rules in find_in_path(). For example, it
> should use first_path_var_separator() to get the platform-specific path
> separator, and probably also substitute_path_macro() and
> canonicalize_path() etc., to keep everything consistent.
>
I fixed this hardcoded path separator issue on the TAP test and forgot
to fix it also on code, sorry, fixed on this new version.
I also spent some time investigating why the tests on Windows were still
passing even using a wrong path separator.
Consider extension_control_path = '$system;C:\custom\path'
When running on Windows, the get_extension_control_directories was
returning [$system;C:, \custom\path] and for somehow the \custom\path
was successfully being read and since the tests was only referencing the
extension on this custom path everything was passing, but querying for
an extension that is only on $system was resulting in an empty query
result. In the attached patch I also included a new test case to query
on pg_available_extensions for an extension that is installed on the
$system, so we can ensure that extensions in both paths can be used
correctly.
> (Maybe it would be ok to move the function to dfmgr.c to avoid having
> to export too many things from there.)
>
I've exported substitute_path_macro because adding a new function on
dfmgr would require #include nodes/pg_list.h and I'm not sure what
approach would be better, please let me know what you think.
--
Matheus Alcantara
