Include extension path on pg_available_extensions

On 9/16/25 8:18 AM, Matheus Alcantara wrote:

Any opinions on this?

[1] /messages/by-id/CAKFQuwbR1Fzr8yRuMW=N1UMA1cTpFcqZe9bW_-ZF8=Ba2Ud2=w@mail.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] /messages/by-id/CAKFQuwbR1Fzr8yRuMW=N1UMA1cTpFcqZe9bW_-ZF8=Ba2Ud2=w@mail.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 a 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);
```

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] /messages/by-id/CAKFQuwbR1Fzr8yRuMW=N1UMA1cTpFcqZe9bW_-ZF8=Ba2Ud2=w@mail.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] /messages/by-id/CAKFQuwbR1Fzr8yRuMW=N1UMA1cTpFcqZe9bW_-ZF8=Ba2Ud2=w@mail.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]https://www.postgresql.org/docs/18/runtime-config-client.html#GUC-EXTENSION-CONTROL-PATH 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 of a 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] /messages/by-id/CAKFQuwbR1Fzr8yRuMW=N1UMA1cTpFcqZe9bW_-ZF8=Ba2Ud2=w@mail.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.

On Thu Oct 23, 2025 at 10:57 PM -03, Quan Zongliang wrote:

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.

So here it is, see attached.

I've created a new role pg_read_extension_paths for this, I'm not sure
if it's the best way to do this. I'm open for other ideas, perhaps we
can reuse some other role?

--
Matheus Alcantara

On Tue, Oct 28, 2025, at 9:29 AM, Matheus Alcantara wrote:

So here it is, see attached.

I took another look at this patch.

! This adds a new "location" column on pg_available_extensions and
! pg_available_extension_versions views to show the path of locations that
! Postgres is seeing based on the extension_control_path GUC.

Maybe the sentence above can be written in a different way such as

Add a new "location" column to pg_available_extensions and
pg_available_extension_versions views. It exposes the directory that the
extension is located.

! User configured paths is only visible for users that has the
! pg_read_extension_paths role, otherwise <insufficient privilege> is
! returned as a column value, the same behaviour that we already have on
! pg_stat_activity.

s/User configured paths is/User-defined locations are/

+typedef struct
+{
+	char	   *macro;
+	char	   *loc;
+} Location;

Location is a generic name. I would use something like ExtensionLocation or
ExtLocation or ExtControlPath. Do you really need a struct here? You are
storing the same element (location) in both members. Couldn't you use a single
string to represent the location (with and without the macro)?

+/*
+ *  Return the location to display for the given Location based on the user
+ *  privileges. If the user connected to the database don't have the
+ *  permissions <insufficient privilege> is returned.
+ */
+static char *
+location_to_display(Location *loc)
+{

Could you improve this comment?

Return the extension location. If the current user doesn't have sufficient
privileges, don't show the location. You could also rename this function
(something like get_extension_location). Since has_privs_of_role returns a
bool, you can simplify the code and call it directly into the if block. You can
also use GetUserId() directly instead of declaring another variable.

+{ oid => '6434', oid_symbol => 'ROLE_PG_READ_EXTENSION_PATHS',
+  rolname => 'pg_read_extension_paths', rolsuper => 'f', rolinherit => 't',
+  rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+  rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
+  rolpassword => '_null_', rolvaliduntil => '_null_' },

I'm not convinced that we need a new predefined role just to control if it can
expose the extension location. Should it return the location only for
superusers? Can't one of the current predefined roles be used? If it doesn't
fit, you should probably use a generic name so this new predefined role can be
used by other extension-related stuff in the future.

@@ -43,31 +51,63 @@ is($ecp, "\$system$sep$ext_dir$sep$ext_dir2",
$node->safe_psql('postgres', "CREATE EXTENSION $ext_name");
$node->safe_psql('postgres', "CREATE EXTENSION $ext_name2");

+
my $ret = $node->safe_psql('postgres',
"select * from pg_available_extensions where name = '$ext_name'");

Remove this new line.

Adding more tests is always a good thing. However, in this case, we can
simplify the tests. The current queries already cover the
get-the-extension-location case. If you add an additional test showing the
insufficient privilege case, that's fine. The other tests are basically
exercising the permission system.

Documentation is missing. These views are documented in system-views.sgml.

--
Euler Taveira
EDB https://www.enterprisedb.com/

On Tue Oct 28, 2025 at 5:56 PM -03, Euler Taveira wrote:

On Tue, Oct 28, 2025, at 9:29 AM, Matheus Alcantara wrote:

So here it is, see attached.

I took another look at this patch.

Thanks for reviewing!

! This adds a new "location" column on pg_available_extensions and
! pg_available_extension_versions views to show the path of locations that
! Postgres is seeing based on the extension_control_path GUC.

Maybe the sentence above can be written in a different way such as

Add a new "location" column to pg_available_extensions and
pg_available_extension_versions views. It exposes the directory that the
extension is located.

Sounds better, fixed.

! User configured paths is only visible for users that has the
! pg_read_extension_paths role, otherwise <insufficient privilege> is
! returned as a column value, the same behaviour that we already have on
! pg_stat_activity.

s/User configured paths is/User-defined locations are/

Fixed.

+typedef struct
+{
+	char	   *macro;
+	char	   *loc;
+} Location;

Location is a generic name. I would use something like ExtensionLocation or
ExtLocation or ExtControlPath. Do you really need a struct here? You are
storing the same element (location) in both members. Couldn't you use a single
string to represent the location (with and without the macro)?

ExtensionLocation sounds good, fixed for this.

I think that the struct is necessary because we use the "loc" field for
other things other than just printing on "location" column results. For
instance, on pg_available_extension_versions() we get the list of
ExtensionLocation's and use the "loc" field to call AllocateDir() and
ReadDir() and then we pass the location pointer to
get_available_versions_for_extension() that it will decide if it should
use the "loc" or the "macro" field by calling the location_to_display().
So I don't think that we can use a single string to represent the
location because we may use the raw location or the macro value
depending on the case.

+/*
+ *  Return the location to display for the given Location based on the user
+ *  privileges. If the user connected to the database don't have the
+ *  permissions <insufficient privilege> is returned.
+ */
+static char *
+location_to_display(Location *loc)
+{

Could you improve this comment?

Fixed.

Return the extension location. If the current user doesn't have sufficient
privileges, don't show the location. You could also rename this function
(something like get_extension_location). Since has_privs_of_role returns a
bool, you can simplify the code and call it directly into the if block. You can
also use GetUserId() directly instead of declaring another variable.

Fixed.

+{ oid => '6434', oid_symbol => 'ROLE_PG_READ_EXTENSION_PATHS',
+  rolname => 'pg_read_extension_paths', rolsuper => 'f', rolinherit => 't',
+  rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+  rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
+  rolpassword => '_null_', rolvaliduntil => '_null_' },

I'm not convinced that we need a new predefined role just to control if it can
expose the extension location. Should it return the location only for
superusers? Can't one of the current predefined roles be used? If it doesn't
fit, you should probably use a generic name so this new predefined role can be
used by other extension-related stuff in the future.

Yeah, I think that we can limit this only for superusers. Fixed.

@@ -43,31 +51,63 @@ is($ecp, "\$system$sep$ext_dir$sep$ext_dir2",
$node->safe_psql('postgres', "CREATE EXTENSION $ext_name");
$node->safe_psql('postgres', "CREATE EXTENSION $ext_name2");

+
my $ret = $node->safe_psql('postgres',
"select * from pg_available_extensions where name = '$ext_name'");

Remove this new line.

Fixed.

Adding more tests is always a good thing. However, in this case, we can
simplify the tests. The current queries already cover the
get-the-extension-location case. If you add an additional test showing the
insufficient privilege case, that's fine. The other tests are basically
exercising the permission system.

Fixed.

Documentation is missing. These views are documented in system-views.sgml.

Fixed

--
Matheus Alcantara

Hi,

On Mon, Sep 15, 2025 at 09:18:25PM -0300, Matheus Alcantara wrote:

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 am not sure just adding the column at the end is best, I would have
put it before comment so that stays last, maybe somebody else has some
bikeshedding input here?

Michae

Thanks for reviewing this!

On Sun Nov 2, 2025 at 12:11 PM -03, Michael Banck wrote:

On Mon, Sep 15, 2025 at 09:18:25PM -0300, Matheus Alcantara wrote:

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 am not sure just adding the column at the end is best, I would have
put it before comment so that stays last, maybe somebody else has some
bikeshedding input here?

Yeah, I think that it looks better to keep the comment at the end. If no
objections I'll swap the order of "comment" and "location" columns on
the next version.

--
Matheus Alcantara
EDB: http://www.enterprisedb.com

On Thu, Nov 6, 2025 at 9:29 AM Matheus Alcantara <matheusssilv97@gmail.com>
wrote:

Thanks for reviewing this!

On Sun Nov 2, 2025 at 12:11 PM -03, Michael Banck wrote:

On Mon, Sep 15, 2025 at 09:18:25PM -0300, Matheus Alcantara wrote:

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 am not sure just adding the column at the end is best, I would have
put it before comment so that stays last, maybe somebody else has some
bikeshedding input here?

Yeah, I think that it looks better to keep the comment at the end. If no
objections I'll swap the order of "comment" and "location" columns on
the next version.

--
Matheus Alcantara
EDB: http://www.enterprisedb.com

Hello!

I have a small bikeshedding comment around making "location" the 4th column
returned for "select * from pg_available_extensions", as opposed to leaving
"comment" the 4th column returned for "select * from
pg_available_extensions".

If a bit of software runs "select * from pg_available_extensions" and
fetches the contents of the 4th column, that column will return "comment"
for current versions of postgres but "location" for patched versions of
postgres.

In many ways, this could be considered a feature and not a bug, because we
should be encouraged to write our SQL like so:

select name, default_version, installed_version, comment from
pg_available_extensions

and not like so:

select * from pg_available_extensions

I'm curious to know if this is a legitimate consideration or not.

Also, there were no surprises when I compiled and tested this: the location
shows correctly for a superuser, and "<insufficient privilege>" shows
correctly for a non-superuser.
--
-- Manni Wood EDB: https://www.enterprisedb.com

Thank you for reviewing this!

On Mon Nov 10, 2025 at 3:25 PM -03, Manni Wood wrote:

Hello!

I have a small bikeshedding comment around making "location" the 4th column
returned for "select * from pg_available_extensions", as opposed to leaving
"comment" the 4th column returned for "select * from
pg_available_extensions".

If a bit of software runs "select * from pg_available_extensions" and
fetches the contents of the 4th column, that column will return "comment"
for current versions of postgres but "location" for patched versions of
postgres.

In many ways, this could be considered a feature and not a bug, because we
should be encouraged to write our SQL like so:

select name, default_version, installed_version, comment from
pg_available_extensions

and not like so:

select * from pg_available_extensions

I'm curious to know if this is a legitimate consideration or not.

Also, there were no surprises when I compiled and tested this: the location
shows correctly for a superuser, and "<insufficient privilege>" shows
correctly for a non-superuser.

Good point, I think that it's a legitimate consideration. That being
said I would get back to prefer to keep the location as the latest
column to avoid such issues even if SELECT * is not something that users
should do in practice, but I think that it's worth to avoid break any
application with such change.

--
Matheus Alcantara
EDB: http://www.enterprisedb.com

Hi,

On Mon, Nov 10, 2025 at 07:48:03PM -0300, Matheus Alcantara wrote:

On Mon Nov 10, 2025 at 3:25 PM -03, Manni Wood wrote:

I have a small bikeshedding comment around making "location" the 4th column
returned for "select * from pg_available_extensions", as opposed to leaving
"comment" the 4th column returned for "select * from
pg_available_extensions".

If a bit of software runs "select * from pg_available_extensions" and
fetches the contents of the 4th column, that column will return "comment"
for current versions of postgres but "location" for patched versions of
postgres.

In many ways, this could be considered a feature and not a bug, because we
should be encouraged to write our SQL like so:

select name, default_version, installed_version, comment from
pg_available_extensions

and not like so:

select * from pg_available_extensions

I'm curious to know if this is a legitimate consideration or not.

Also, there were no surprises when I compiled and tested this: the location
shows correctly for a superuser, and "<insufficient privilege>" shows
correctly for a non-superuser.

Good point, I think that it's a legitimate consideration. That being
said I would get back to prefer to keep the location as the latest
column to avoid such issues even if SELECT * is not something that users
should do in practice, but I think that it's worth to avoid break any
application with such change.

When the trusted column got added to the pg_availe_extensions view in
50fc694, it wasn't added to the end, but next to superuser, where it
logically makes sense IMO:

|@@ -317,7 +317,8 @@ CREATE VIEW pg_available_extensions AS
|
| CREATE VIEW pg_available_extension_versions AS
| SELECT E.name, E.version, (X.extname IS NOT NULL) AS installed,
|- E.superuser, E.relocatable, E.schema, E.requires, E.comment
|+ E.superuser, E.trusted, E.relocatable,
|+ E.schema, E.requires, E.comment
| FROM pg_available_extension_versions() AS E
| LEFT JOIN pg_extension AS X
| ON E.name = X.extname AND E.version = X.extversion;

As far as I know, Postgres does not guarantee stable system catalogs
between major versions, so I don't think users should or could rely on
stable column ordering, really.

Michael

Hi Michael,

I am just getting started in the Postgres community (this is my first code review). So please excuse me if I have missed something (in terms of process etc).
I reviewed your proposed code changes in the attached patch file and they look good to me. I have some minor comments:

1. In src/test/modules/test_extensions/t/001_extension_control_path.pl, it would be nice if you could add a test that validates that the correct Extension location is displayed, if for example, the extension is being picked up from a customized location.
2. Nit-pick: In src/backend/commands/extension.c:get_available_versions_for_extension(), you could probably combine the following 2 lines into one to say "'comment' & 'location' stay the same.
/* comment stays the same */
/* location stays the same */

Hope this is helpful.

Thanks,
-Rohit

On Mon Nov 10, 2025 at 8:10 PM -03, Michael Banck wrote:

I have a small bikeshedding comment around making "location" the 4th column
returned for "select * from pg_available_extensions", as opposed to leaving
"comment" the 4th column returned for "select * from
pg_available_extensions".

If a bit of software runs "select * from pg_available_extensions" and
fetches the contents of the 4th column, that column will return "comment"
for current versions of postgres but "location" for patched versions of
postgres.

In many ways, this could be considered a feature and not a bug, because we
should be encouraged to write our SQL like so:

select name, default_version, installed_version, comment from
pg_available_extensions

and not like so:

select * from pg_available_extensions

I'm curious to know if this is a legitimate consideration or not.

Good point, I think that it's a legitimate consideration. That being
said I would get back to prefer to keep the location as the latest
column to avoid such issues even if SELECT * is not something that users
should do in practice, but I think that it's worth to avoid break any
application with such change.

When the trusted column got added to the pg_availe_extensions view in
50fc694, it wasn't added to the end, but next to superuser, where it
logically makes sense IMO:

|@@ -317,7 +317,8 @@ CREATE VIEW pg_available_extensions AS
|
| CREATE VIEW pg_available_extension_versions AS
| SELECT E.name, E.version, (X.extname IS NOT NULL) AS installed,
|- E.superuser, E.relocatable, E.schema, E.requires, E.comment
|+ E.superuser, E.trusted, E.relocatable,
|+ E.schema, E.requires, E.comment
| FROM pg_available_extension_versions() AS E
| LEFT JOIN pg_extension AS X
| ON E.name = X.extname AND E.version = X.extversion;

As far as I know, Postgres does not guarantee stable system catalogs
between major versions, so I don't think users should or could rely on
stable column ordering, really.

Thanks for pointing this, I didn't know about this detail. I'm not
against swapping the orders of "comment" and "location" columns, I also
think that it would look better, I'm just afraid of breaking any
compatibility with anything, but it seems that it's not the case.

--
Matheus Alcantara
EDB: http://www.enterprisedb.com

On Mon Nov 10, 2025 at 11:06 PM -03, Rohit Prasad wrote:

Hi Michael,

I think you wanted to say Matheus :)

I am just getting started in the Postgres community (this is my first
code review). So please excuse me if I have missed something (in terms
of process etc).

Thank you for reviewing this!

I reviewed your proposed code changes in the attached patch file and
they look good to me.

Thanks.

I have some minor comments:
1. In src/test/modules/test_extensions/t/001_extension_control_path.pl,
it would be nice if you could add a test that validates that the
correct Extension location is displayed, if for example, the extension
is being picked up from a customized location.

I don't know if I get your point here. On the v4 patch we have:
-       "test_custom_ext_paths|1.0|1.0|Test extension_control_path",
+       "test_custom_ext_paths|1.0|1.0|Test extension_control_path|$ext_dir_canonicalized/extension",

The $ext_dir_canonicalized in this case is a custom path configured on
extension_control_path GUC that the "test_custom_ext_paths" extension
was installed.

2. Nit-pick: In
src/backend/commands/extension.c:get_available_versions_for_extension(),
you could probably combine the following 2 lines into one to say
"'comment' & 'location' stay the same.
/* comment stays the same */
/* location stays the same */

Fixed on attached v5

On this new v5 version I also swap the order of "comment" and "location"
columns as it was suggested by Michael.

--
Matheus Alcantara
EDB: http://www.enterprisedb.com

Hi Matheus,

Apologies for the mix up on the names :-)

I agree with your responses (please ignore my prior comment about test_extension code; I had missed some of your changes there).

I also reviewed v5 of your patch and the changes look good to me.

Thanks,
-Rohit