Feedback-ID: ie0a040ee:Fastmail
Message-ID: <28e98657-9369-4539-9e06-9124d1e77853@eisentraut.org>
Date: Wed, 19 Mar 2025 07:42:44 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: RFC: Additional Directory for Extensions
To: Matheus Alcantara <matheusssilv97@gmail.com>
Cc: Andrew Dunstan <andrew@dunslane.net>,
 Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>,
 Craig Ringer <craig.ringer@enterprisedb.com>,
 "David E. Wheeler" <david@justatheory.com>,
 Robert Haas <robertmhaas@gmail.com>, Alvaro Herrera
 <alvherre@alvh.no-ip.org>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
References: 
 <CA+TgmoaMk==vOUmWwJ5ZA-EueNqa9RQaWMEiHCF1ZKW_YCcj8g@mail.gmail.com>
 <AF4A50CD-C5CF-406D-9700-1B889F40E8FB@justatheory.com>
 <CAGRY4nx8Fmea4udk--PKdvfgpNdqC+rcueAmGwuw2PD9dv+zqQ@mail.gmail.com>
 <D30A91FA-A6D4-4737-941F-0BBB2984B730@justatheory.com>
 <0d384836-7e6e-4932-af3b-8dad1f6fee43@eisentraut.org>
 <4f7209ba-1ca2-475e-9a70-0f3fe0be6378@eisentraut.org>
 <CAFY6G8dzuLXFjONX5E-TvyD5E9r21b5vp1My_nMcqdGD0VYfYw@mail.gmail.com>
 <12312928-5516-49b8-81c1-a7b56fd01bc8@dunslane.net>
 <CAFY6G8dGcv1D8tCkjrz+9DvAYG9h4U=hBm0VXjJEXExy44hauQ@mail.gmail.com>
 <CAFY6G8eOMkGXLqzUz=qXrwt4X_HvmVZ13570DAbgAiw+3q03DA@mail.gmail.com>
 <CAFY6G8fV5FTqn7p8pxbvPYUD5+ffik-J50GChjKH2Gg7d=fRPg@mail.gmail.com>
 <38c53446-6b65-4222-b76b-351e56f4841c@eisentraut.org>
 <CAFY6G8cGeUV0f5K8v-Du0ts3iZyRE6Q5dNtYjQq8cjS4epLX5A@mail.gmail.com>
 <7c178ff3-20a5-493e-a704-e6b3263e782f@eisentraut.org>
 <CAFY6G8d56BK5TZ7K+uw4DHLZ=9Th3p+E1x4pp26nJ_K5k4EUwA@mail.gmail.com>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: 
 <CAFY6G8d56BK5TZ7K+uw4DHLZ=9Th3p+E1x4pp26nJ_K5k4EUwA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/28e98657-9369-4539-9e06-9124d1e77853%40eisentraut.org>
Precedence: bulk

On 12.03.25 14:17, Matheus Alcantara wrote:
>> 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.
>>
> Good catch. I fixed this by creating a new function to construct the
> ExtensionControlFile and changed the pg_available_extensions to set the
> control_dir. The read_extension_control_file was also changed to just
> call this new function constructor. I implemented the logic to check if
> the control_dir is already set on parse_extension_control_file because
> it seems to me that make more sense to not call
> find_extension_control_filename instead of putting this logic there
> since we already set the control_dir when we find the control file, and
> having the logic to set the control_dir or skip the find_in_path seems
> more confusing on this function instead of on
> parse_extension_control_file. Please let me know what you think.

Committed that, thanks.

A small tweak I made was to replace palloc+snprintf by psprintf.  Maybe 
you were not aware that that function exists.

I also simplified the error handling in parse_extension_control_file() a 
bit.  If we pass in a control directory (which is the new code we're 
adding), then we can assume that we already found the file earlier, and 
then if we now don't find it, then we should just report the file system 
error instead of the "you should install this extension first" error. 
It's kind of a "can't happen" error anyway, so the different is small.