Re: RFC: Additional Directory for Extensions

On 10.03.25 21:25, Matheus Alcantara wrote:
> 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.

>> (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.

Yes, that structure looks ok.  But you can remove one level of block in 
get_extension_control_directories().

I found a bug that was already present in my earlier patch versions:

@@ -423,7 +424,7 @@ find_extension_control_filename(ExtensionControlFile 
*control)
     ecp = Extension_control_path;
     if (strlen(ecp) == 0)
         ecp = "$system";
-   result = find_in_path(basename, Extension_control_path, 
"extension_control_path", "$system", system_dir);
+   result = find_in_path(basename, ecp, "extension_control_path", 
"$system", system_dir);

Without this, it won't work if you set extension_control_path empty. 
(Maybe add a test for that?)

I think this all works now, but I think the way 
pg_available_extensions() works is a bit strange and inefficient.  After 
it finds a candidate control file, it calls 
read_extension_control_file() with the extension name, that calls 
parse_extension_control_file(), that calls 
find_extension_control_filename(), and that calls find_in_path(), which 
searches that path again!

There should be a simpler way into this.  Maybe 
pg_available_extensions() should fill out the ExtensionControlFile 
structure itself, set ->control_dir with where it found it, then call 
directly to parse_extension_control_file(), and that should skip the 
finding if the directory is already set.  Or something similar.