Обсуждение: Include extension path on pg_available_extensions
Hi all,
On [1] it was mentioned that it could be a good idea to include the
extension location when listening the available extensions on
pg_available_extensions to make it clear to the user the location of an
extension that Postgres is seeing based on the extension_control_path
GUC.
The attached patch implements this idea. Extensions installed on $system
path will not show the actual value of the $system macro and it will
show the macro itself, for example:
postgres=# show extension_control_path;
extension_control_path
---------------------------------------------------
/usr/local/my/extensions/share/postgresql:$system
(1 row)
postgres=# select * from pg_available_extensions;
name | default_version | installed_version | comment |
location
---------+-----------------+-------------------+--------------------------------------------------+---------------------------------------------------
envvar | 1.0.0 | | Get the value of a server environment variable |
/usr/local/my/extensions/share/postgresql/extension
amcheck | 1.5 | | functions for verifying relation integrity | $system
bloom | 1.0 | | bloom access method - signature file based index | $system
I'm not sure if this should be included on 18 release since this is not
a bug fix but an improvement on the extension system by itself.
Any opinions on this?
[1] https://www.postgresql.org/message-id/CAKFQuwbR1Fzr8yRuMW%3DN1UMA1cTpFcqZe9bW_-ZF8%3DBa2Ud2%3Dw%40mail.gmail.com
--
Matheus Alcantara
Вложения
On 9/16/25 8:18 AM, Matheus Alcantara wrote: > Any opinions on this? > > [1] https://www.postgresql.org/message-id/CAKFQuwbR1Fzr8yRuMW%3DN1UMA1cTpFcqZe9bW_-ZF8%3DBa2Ud2%3Dw%40mail.gmail.com > Just as the discussion here. Adding extension location is a good idea. Suppose there is an amcheck 1.5 located in the $system directory. There is also an amcheck 1.4.5 located in another path. Strange results will then occur: postgres=# SHOW extension_control_path; extension_control_path ------------------------ $system (1 row) postgres=# CREATE EXTENSION amcheck; CREATE EXTENSION postgres=# select * from pg_available_extensions; name | default_version | installed_version | comment | location ------------+-----------------+-------------------+--------------------------------------------+---------- amcheck | 1.5 | 1.5 | functions for verifying relation integrity | $system This seems to be fine. However, if another path is added, strange results will occur. postgres=# SET extension_control_path TO '/Users/quanzl/build/pg-availext:$system'; SET postgres=# select * from pg_available_extensions; name | default_version | installed_version | comment | location ------------+-----------------+-------------------+--------------------------------------------+------------------------------------------- amcheck | 1.4.5 | 1.5 | functions for verifying relation integrity | /Users/quanzl/build/pg-availext/extension The results shown here will cause confusion. It is better to show the path used at creation. So, it would be a better option to add a new column to the pg_extension table. -- Quan Zongliang
> On 9/16/25 8:18 AM, Matheus Alcantara wrote: > >> Any opinions on this? >> [1] https://www.postgresql.org/message-id/CAKFQuwbR1Fzr8yRuMW%3DN1UMA1cTpFcqZe9bW_-ZF8%3DBa2Ud2%3Dw%40mail.gmail.com > Just as the discussion here. Adding extension location is a good idea. +1. I like the ideal. > -- > Matheus Alcantara > <v1-0001-Add-path-of-extension-on-pg_available_extensions.patch> Got a few comments: 1 - extension.c ``` +/* + * A location configured on extension_control_path GUC. + * + * The macro is the macro plaeholder that the extension_control_path support + * and which is replaced by a system value that is stored on loc. For custom + * paths that don't have a macro the macro field is NULL. + */ ``` Some problems in the comment: * typo: plaebholder -> placeholder * "the extension_control_path support” where “support” should be “supports” * “stored on loc” should be “stored in loc” Also, “The macro is the macro placeholder …” sounds redundant, suggested revision: “The macro field stores the name of amacro (for example “$system”) that extension_control_path supports, which is replaced by …" 2 - extension.c ``` + Location *location = palloc0_object(Location); + + location->macro = NULL; + location->loc = system_dir; + paths = lappend(paths, location); ``` As you immediately assign values to all fields, palloc0_object() is not needed, palloc_object() is good enough. 3 - extension.c ``` @@ -366,6 +384,7 @@ get_extension_control_directories(void) int len; char *mangled; char *piece = first_path_var_separator(ecp); + Location *location = palloc0_object(Location); ``` In all execution paths, location will be initiated, thus palloc_object() is good enough. 4 - extension.c ``` + /* location */ + if (location->macro == NULL) + values[3] = CStringGetTextDatum(location->loc); + else + values[3] = CStringGetTextDatum(location->macro); ``` There are multiple places of this “if-else”. So, “macro” basically is for display, and loc is the real location. I am thinking,maybe we can change the definition of Location to: ``` structure Location { Char *display; Char *real; ``` When it is not a macro, just assign real to display, so that we can avoid all these “if-else”. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
On Wed, Oct 22, 2025, at 10:28 PM, Chao Li wrote: >> On 9/16/25 8:18 AM, Matheus Alcantara wrote: >> >>> Any opinions on this? >>> [1] https://www.postgresql.org/message-id/CAKFQuwbR1Fzr8yRuMW%3DN1UMA1cTpFcqZe9bW_-ZF8%3DBa2Ud2%3Dw%40mail.gmail.com >> Just as the discussion here. Adding extension location is a good idea. > > > +1. I like the ideal. > Exposing useful information might be a good idea except if it doesn't compromise security. IIRC there is no function or view that exposes absolute path to regular users. The view pg_available_extensions has PUBLIC access. Check similar functions using a query like: SELECT proname, x.unnest AS argname FROM (SELECT proname, unnest(proargnames) FROM pg_proc) AS x WHERE x.unnest ~ 'file' OR x.unnest ~ 'path'; Some of the functions that return absolute path revoked PUBLIC access for security reason. See pg_show_all_file_settings, pg_hba_file_rules, and pg_ident_file_mappings. (All of these functions have a view that returns its content similar to pg_available_extensions.) See system_views.sql. Do we want to use a similar pattern (revoke PUBLIC access from the function)? It breaks the compatibility but perhaps using an existent pre-defined role (pg_read_all_settings?) may be less harmful. There are at least 2 alternatives: * separate function: add a new function that returns the absolute path. Don't grant PUBLIC access. It doesn't break compatibility but you need to modify your query. * insufficient privilege: if the role doesn't have the sufficient privileges, return NULL or '<insufficient privilege>' (similar to pg_stat_activity). I don't have a strong preference but the latter can impose more effort to use if you don't know the role has sufficient privilege. However, it is clear why the absolute path is not returned. -- Euler Taveira EDB https://www.enterprisedb.com/
Thanks for testing this! On Wed Oct 22, 2025 at 9:19 PM -03, Quan Zongliang wrote: > On 9/16/25 8:18 AM, Matheus Alcantara wrote: > >> Any opinions on this? >> >> [1] https://www.postgresql.org/message-id/CAKFQuwbR1Fzr8yRuMW%3DN1UMA1cTpFcqZe9bW_-ZF8%3DBa2Ud2%3Dw%40mail.gmail.com >> > Just as the discussion here. Adding extension location is a good idea. > Suppose there is an amcheck 1.5 located in the $system directory. There > is also an amcheck 1.4.5 located in another path. > > Strange results will then occur: > postgres=# SHOW extension_control_path; > extension_control_path > ------------------------ > $system > (1 row) > > postgres=# CREATE EXTENSION amcheck; > CREATE EXTENSION > postgres=# select * from pg_available_extensions; > > name | default_version | > installed_version | comment | location > ------------+-----------------+-------------------+--------------------------------------------+---------- > amcheck | 1.5 | 1.5 | functions for > verifying relation integrity | $system > > This seems to be fine. > > However, if another path is added, strange results will occur. > > postgres=# SET extension_control_path TO > '/Users/quanzl/build/pg-availext:$system'; > SET > postgres=# select * from pg_available_extensions; > name | default_version | installed_version | > comment | location > ------------+-----------------+-------------------+--------------------------------------------+------------------------------------------- > amcheck | 1.4.5 | 1.5 | functions for > verifying relation integrity | /Users/quanzl/build/pg-availext/extension > > The results shown here will cause confusion. It is better to show the > path used at creation. > I agree that this sounds strange but the documentation [1] mention the following: If extensions with equal names are present in multiple directories in the configured path, only the instance found first in the path will be used. So I think that users should not use different paths to install the same extension with different versions in practice. > So, it would be a better option to add a new column to the pg_extension > table. > You mean add the location column on pg_extension instead of pg_available_extensions? I'm not sure if I get the point here. [1] https://www.postgresql.org/docs/18/runtime-config-client.html#GUC-EXTENSION-CONTROL-PATH -- Matheus Alcantara
Thanks for reviewing this!
On Wed Oct 22, 2025 at 10:28 PM -03, Chao Li wrote:
>> <v1-0001-Add-path-of-extension-on-pg_available_extensions.patch>
>
> Got a few comments:
>
> 1 - extension.c
> ```
> +/*
> + * A location configured on extension_control_path GUC.
> + *
> + * The macro is the macro plaeholder that the extension_control_path support
> + * and which is replaced by a system value that is stored on loc. For custom
> + * paths that don't have a macro the macro field is NULL.
> + */
> ```
>
> Some problems in the comment:
>
> * typo: plaebholder -> placeholder
> * "the extension_control_path support” where “support” should be “supports”
> * “stored on loc” should be “stored in loc”
>
Fixed
> Also, “The macro is the macro placeholder …” sounds redundant, suggested revision: “The macro field stores the name
ofa macro (for example “$system”) that extension_control_path supports, which is replaced by …"
>
> 2 - extension.c
> ```
> + Location *location = palloc0_object(Location);
> +
> + location->macro = NULL;
> + location->loc = system_dir;
> + paths = lappend(paths, location);
> ```
>
Fixed
> As you immediately assign values to all fields, palloc0_object() is not needed, palloc_object() is good enough.
>
> 3 - extension.c
> ```
> @@ -366,6 +384,7 @@ get_extension_control_directories(void)
> int len;
> char *mangled;
> char *piece = first_path_var_separator(ecp);
> + Location *location = palloc0_object(Location);
> ```
>
> In all execution paths, location will be initiated, thus palloc_object() is good enough.
>
Fixed
> 4 - extension.c
> ```
> + /* location */
> + if (location->macro == NULL)
> + values[3] = CStringGetTextDatum(location->loc);
> + else
> + values[3] = CStringGetTextDatum(location->macro);
> ```
>
> There are multiple places of this “if-else”. So, “macro” basically is for display, and loc is the real location. I am
thinking,maybe we can change the definition of Location to:
>
> ```
> structure Location {
> Char *display;
> Char *real;
> ```
>
> When it is not a macro, just assign real to display, so that we can avoid all these “if-else”.
>
These struct fields sounds a bit unclear by just looking it without
reading the usages to me TBH. What do you think by creating a static
function that do the if-else and just use it? Perhaps make into a macro?
Attached v2 with all the fixes.
--
Matheus Alcantara
Вложения
On Thu Oct 23, 2025 at 10:56 AM -03, Euler Taveira wrote: > On Wed, Oct 22, 2025, at 10:28 PM, Chao Li wrote: >>> On 9/16/25 8:18 AM, Matheus Alcantara wrote: >>> >>>> Any opinions on this? >>>> [1] https://www.postgresql.org/message-id/CAKFQuwbR1Fzr8yRuMW%3DN1UMA1cTpFcqZe9bW_-ZF8%3DBa2Ud2%3Dw%40mail.gmail.com >>> Just as the discussion here. Adding extension location is a good idea. >> >> >> +1. I like the ideal. >> > > Exposing useful information might be a good idea except if it doesn't > compromise security. IIRC there is no function or view that exposes absolute > path to regular users. > > The view pg_available_extensions has PUBLIC access. Check similar functions > using a query like: > > SELECT proname, > x.unnest AS argname > FROM > (SELECT proname, > unnest(proargnames) > FROM pg_proc) AS x > WHERE x.unnest ~ 'file' > OR x.unnest ~ 'path'; > > Some of the functions that return absolute path revoked PUBLIC access for > security reason. See pg_show_all_file_settings, pg_hba_file_rules, and > pg_ident_file_mappings. (All of these functions have a view that returns its > content similar to pg_available_extensions.) See system_views.sql. > > Do we want to use a similar pattern (revoke PUBLIC access from the function)? > It breaks the compatibility but perhaps using an existent pre-defined role > (pg_read_all_settings?) may be less harmful. > > There are at least 2 alternatives: > > * separate function: add a new function that returns the absolute path. Don't > grant PUBLIC access. It doesn't break compatibility but you need to modify > your query. > > * insufficient privilege: if the role doesn't have the sufficient privileges, > return NULL or '<insufficient privilege>' (similar to pg_stat_activity). I > don't have a strong preference but the latter can impose more effort to use > if you don't know the role has sufficient privilege. However, it is clear why > the absolute path is not returned. > Yeah, I agree. I've implemented the first version in a way it doesn't show the real value of the $system macro because of security reasons but I agree that we don't want that any user can see the configured path of custom extensions too. I would prefer to go with the '<insufficient privilege>' since it's something that we already have today in other views and users may already know how to deal with it. -- Matheus Alcantara
On 10/23/25 9:56 PM, Euler Taveira wrote: > > * insufficient privilege: if the role doesn't have the sufficient privileges, > return NULL or '<insufficient privilege>' (similar to pg_stat_activity). I > don't have a strong preference but the latter can impose more effort to use > if you don't know the role has sufficient privilege. However, it is clear why > the absolute path is not returned. > +1 I think this way is better.