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
From fa6560a81c746ea28c12256fcbe0372850385332 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <[email protected]>
Date: Mon, 15 Sep 2025 15:46:24 -0300
Subject: [PATCH v4] Add path of extension on pg_available_extensions
Add a new "location" column to pg_available_extensions and
pg_available_extension_versions views. It exposes the directory that the
extension is located.
The default system location is show as $system macro, the same value
that is used to configure the extension_control_path GUC.
User-defined locations are 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.
---
doc/src/sgml/system-views.sgml | 22 +++++
src/backend/catalog/system_views.sql | 4 +-
src/backend/commands/extension.c | 95 +++++++++++++++----
src/include/catalog/pg_proc.dat | 10 +-
.../t/001_extension_control_path.pl | 35 ++++++-
src/test/regress/expected/rules.out | 10 +-
src/tools/pgindent/typedefs.list | 2 +-
7 files changed, 142 insertions(+), 36 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 7971498fe75..aef0f067436 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -607,6 +607,17 @@
Comment string from the extension's control file
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>location</structfield> <type>text</type>
+ </para>
+ <para>
+ The location where the extension was found based on the <link
+
linkend="guc-extension-control-path"><structname>extension_control_path</structname></link>
+ GUC. Only superusers can see the contents of this column.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
@@ -731,6 +742,17 @@
Comment string from the extension's control file
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>location</structfield> <type>text</type>
+ </para>
+ <para>
+ The location where the extension was found based on the <link
+
linkend="guc-extension-control-path"><structname>extension_control_path</structname></link>
+ GUC. Only superusers can see the contents of this column.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql
b/src/backend/catalog/system_views.sql
index dec8df4f8ee..8a084927954 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -412,14 +412,14 @@ CREATE VIEW pg_cursors AS
CREATE VIEW pg_available_extensions AS
SELECT E.name, E.default_version, X.extversion AS installed_version,
- E.comment
+ E.comment, E.location
FROM pg_available_extensions() AS E
LEFT JOIN pg_extension AS X ON E.name = X.extname;
CREATE VIEW pg_available_extension_versions AS
SELECT E.name, E.version, (X.extname IS NOT NULL) AS installed,
E.superuser, E.trusted, E.relocatable,
- E.schema, E.requires, E.comment
+ E.schema, E.requires, E.comment, E.location
FROM pg_available_extension_versions() AS E
LEFT JOIN pg_extension AS X
ON E.name = X.extname AND E.version = X.extversion;
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 93ef1ad106f..e64a681790e 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -126,6 +126,20 @@ typedef struct
ParseLoc stmt_len; /* length in bytes; 0 means
"rest of string" */
} script_error_callback_arg;
+/*
+ * A location configured on extension_control_path GUC.
+ *
+ * The macro field stores the name of a macro (for example “$system”) that
+ * extension_control_path supports, which is replaced by a system value that is
+ * stored in loc. For custom paths that don't have a macro the macro field is
+ * NULL.
+ */
+typedef struct
+{
+ char *macro;
+ char *loc;
+} ExtensionLocation;
+
/* Local functions */
static List *find_update_path(List *evi_list,
ExtensionVersionInfo
*evi_start,
@@ -140,7 +154,8 @@ static Oid get_required_extension(char *reqExtensionName,
bool
is_create);
static void get_available_versions_for_extension(ExtensionControlFile
*pcontrol,
Tuplestorestate *tupstore,
-
TupleDesc tupdesc);
+
TupleDesc tupdesc,
+
ExtensionLocation *location);
static Datum convert_requires_to_datum(List *requires);
static void ApplyExtensionUpdates(Oid extensionOid,
ExtensionControlFile *pcontrol,
@@ -157,6 +172,26 @@ static ExtensionControlFile
*new_ExtensionControlFile(const char *extname);
char *find_in_paths(const char *basename, List *paths);
+/*
+ * Return the extension location. If the current user doesn't have sufficient
+ * privileges, don't show the location.
+ */
+static char *
+get_extension_location(ExtensionLocation *loc)
+{
+ /* We only want to show extension paths for superusers. */
+ if (superuser_arg(GetUserId()))
+ {
+ /* Return the macro value if it's present to don't show system
paths. */
+ if (loc->macro == NULL)
+ return loc->loc;
+ else
+ return loc->macro;
+ }
+ else
+ return "<insufficient privilege>";
+}
+
/*
* get_extension_oid - given an extension name, look up the OID
*
@@ -354,7 +389,11 @@ get_extension_control_directories(void)
if (strlen(Extension_control_path) == 0)
{
- paths = lappend(paths, system_dir);
+ ExtensionLocation *location = palloc_object(ExtensionLocation);
+
+ location->macro = NULL;
+ location->loc = system_dir;
+ paths = lappend(paths, location);
}
else
{
@@ -366,6 +405,7 @@ get_extension_control_directories(void)
int len;
char *mangled;
char *piece = first_path_var_separator(ecp);
+ ExtensionLocation *location =
palloc_object(ExtensionLocation);
/* Get the length of the next path on ecp */
if (piece == NULL)
@@ -382,15 +422,21 @@ get_extension_control_directories(void)
* suffix if it is a custom extension control path.
*/
if (strcmp(piece, "$system") == 0)
+ {
+ location->macro = pstrdup(piece);
mangled = substitute_path_macro(piece,
"$system", system_dir);
+ }
else
+ {
+ location->macro = NULL;
mangled = psprintf("%s/extension", piece);
-
+ }
pfree(piece);
/* Canonicalize the path based on the OS and add to the
list */
canonicalize_path(mangled);
- paths = lappend(paths, mangled);
+ location->loc = mangled;
+ paths = lappend(paths, location);
/* Break if ecp is empty or move to the next path on
ecp */
if (ecp[len] == '\0')
@@ -2215,9 +2261,9 @@ pg_available_extensions(PG_FUNCTION_ARGS)
locations = get_extension_control_directories();
- foreach_ptr(char, location, locations)
+ foreach_ptr(ExtensionLocation, location, locations)
{
- dir = AllocateDir(location);
+ dir = AllocateDir(location->loc);
/*
* If the control directory doesn't exist, we want to silently
return
@@ -2229,13 +2275,13 @@ pg_available_extensions(PG_FUNCTION_ARGS)
}
else
{
- while ((de = ReadDir(dir, location)) != NULL)
+ while ((de = ReadDir(dir, location->loc)) != NULL)
{
ExtensionControlFile *control;
char *extname;
String *extname_str;
- Datum values[3];
- bool nulls[3];
+ Datum values[4];
+ bool nulls[4];
if (!is_extension_control_filename(de->d_name))
continue;
@@ -2259,7 +2305,7 @@ pg_available_extensions(PG_FUNCTION_ARGS)
found_ext = lappend(found_ext,
extname_str);
control = new_ExtensionControlFile(extname);
- control->control_dir = pstrdup(location);
+ control->control_dir = pstrdup(location->loc);
parse_extension_control_file(control, NULL);
memset(values, 0, sizeof(values));
@@ -2279,6 +2325,9 @@ pg_available_extensions(PG_FUNCTION_ARGS)
else
values[2] =
CStringGetTextDatum(control->comment);
+ /* location */
+ values[3] =
CStringGetTextDatum(get_extension_location(location));
+
tuplestore_putvalues(rsinfo->setResult,
rsinfo->setDesc,
values, nulls);
}
@@ -2313,9 +2362,9 @@ pg_available_extension_versions(PG_FUNCTION_ARGS)
locations = get_extension_control_directories();
- foreach_ptr(char, location, locations)
+ foreach_ptr(ExtensionLocation, location, locations)
{
- dir = AllocateDir(location);
+ dir = AllocateDir(location->loc);
/*
* If the control directory doesn't exist, we want to silently
return
@@ -2327,7 +2376,7 @@ pg_available_extension_versions(PG_FUNCTION_ARGS)
}
else
{
- while ((de = ReadDir(dir, location)) != NULL)
+ while ((de = ReadDir(dir, location->loc)) != NULL)
{
ExtensionControlFile *control;
char *extname;
@@ -2356,12 +2405,13 @@ pg_available_extension_versions(PG_FUNCTION_ARGS)
/* read the control file */
control = new_ExtensionControlFile(extname);
- control->control_dir = pstrdup(location);
+ control->control_dir = pstrdup(location->loc);
parse_extension_control_file(control, NULL);
/* scan extension's script directory for
install scripts */
get_available_versions_for_extension(control,
rsinfo->setResult,
-
rsinfo->setDesc);
+
rsinfo->setDesc,
+
location);
}
FreeDir(dir);
@@ -2378,7 +2428,8 @@ pg_available_extension_versions(PG_FUNCTION_ARGS)
static void
get_available_versions_for_extension(ExtensionControlFile *pcontrol,
Tuplestorestate *tupstore,
-
TupleDesc tupdesc)
+
TupleDesc tupdesc,
+
ExtensionLocation *location)
{
List *evi_list;
ListCell *lc;
@@ -2391,8 +2442,8 @@ get_available_versions_for_extension(ExtensionControlFile
*pcontrol,
{
ExtensionVersionInfo *evi = (ExtensionVersionInfo *) lfirst(lc);
ExtensionControlFile *control;
- Datum values[8];
- bool nulls[8];
+ Datum values[9];
+ bool nulls[9];
ListCell *lc2;
if (!evi->installable)
@@ -2434,6 +2485,9 @@ get_available_versions_for_extension(ExtensionControlFile
*pcontrol,
else
values[7] = CStringGetTextDatum(control->comment);
+ /* location */
+ values[8] =
CStringGetTextDatum(get_extension_location(location));
+
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
/*
@@ -2475,6 +2529,8 @@ get_available_versions_for_extension(ExtensionControlFile
*pcontrol,
}
/* comment stays the same */
+ /* location stays the same */
+
tuplestore_putvalues(tupstore, tupdesc, values,
nulls);
}
}
@@ -3903,7 +3959,8 @@ find_in_paths(const char *basename, List *paths)
foreach(cell, paths)
{
- char *path = lfirst(cell);
+ ExtensionLocation *location = lfirst(cell);
+ char *path = location->loc;
char *full;
Assert(path != NULL);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 9121a382f76..86d0509f2c7 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10743,16 +10743,16 @@
{ oid => '3082', descr => 'list available extensions',
proname => 'pg_available_extensions', procost => '10', prorows => '100',
proretset => 't', provolatile => 's', prorettype => 'record',
- proargtypes => '', proallargtypes => '{name,text,text}',
- proargmodes => '{o,o,o}', proargnames => '{name,default_version,comment}',
+ proargtypes => '', proallargtypes => '{name,text,text,text}',
+ proargmodes => '{o,o,o,o}', proargnames =>
'{name,default_version,comment,location}',
prosrc => 'pg_available_extensions' },
{ oid => '3083', descr => 'list available extension versions',
proname => 'pg_available_extension_versions', procost => '10',
prorows => '100', proretset => 't', provolatile => 's',
prorettype => 'record', proargtypes => '',
- proallargtypes => '{name,text,bool,bool,bool,name,_name,text}',
- proargmodes => '{o,o,o,o,o,o,o,o}',
- proargnames =>
'{name,version,superuser,trusted,relocatable,schema,requires,comment}',
+ proallargtypes => '{name,text,bool,bool,bool,name,_name,text,text}',
+ proargmodes => '{o,o,o,o,o,o,o,o,o}',
+ proargnames =>
'{name,version,superuser,trusted,relocatable,schema,requires,comment,location}',
prosrc => 'pg_available_extension_versions' },
{ oid => '3084', descr => 'list an extension\'s version update paths',
proname => 'pg_extension_update_paths', procost => '10', prorows => '100',
diff --git a/src/test/modules/test_extensions/t/001_extension_control_path.pl
b/src/test/modules/test_extensions/t/001_extension_control_path.pl
index 7fbe5bde332..498d96969f0 100644
--- a/src/test/modules/test_extensions/t/001_extension_control_path.pl
+++ b/src/test/modules/test_extensions/t/001_extension_control_path.pl
@@ -25,6 +25,10 @@ my $ext_name2 = "test_custom_ext_paths_using_directory";
mkpath("$ext_dir/$ext_name2");
create_extension($ext_name2, $ext_dir, $ext_name2);
+# Make windows path use Unix slashes as canonicalize_path() is called when
+# collecting extension control paths. See get_extension_control_directories().
+my $ext_dir_canonicalized = $windows_os ? ($ext_dir =~ s/\\/\//gr) : $ext_dir;
+
# Use the correct separator and escape \ when running on Windows.
my $sep = $windows_os ? ";" : ":";
$node->append_conf(
@@ -35,6 +39,10 @@ extension_control_path = '\$system$sep@{[ $windows_os ?
($ext_dir =~ s/\\/\\\\/g
# Start node
$node->start;
+# Create an user to test permissions to read extension locations.
+my $user = "user01";
+$node->safe_psql('postgres', "CREATE USER $user");
+
my $ecp = $node->safe_psql('postgres', 'show extension_control_path;');
is($ecp, "\$system$sep$ext_dir$sep$ext_dir2",
@@ -46,28 +54,46 @@ $node->safe_psql('postgres', "CREATE EXTENSION $ext_name2");
my $ret = $node->safe_psql('postgres',
"select * from pg_available_extensions where name = '$ext_name'");
is( $ret,
- "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",
"extension is installed correctly on pg_available_extensions");
$ret = $node->safe_psql('postgres',
"select * from pg_available_extension_versions where name =
'$ext_name'");
is( $ret,
- "test_custom_ext_paths|1.0|t|t|f|t|||Test extension_control_path",
+ "test_custom_ext_paths|1.0|t|t|f|t|||Test
extension_control_path|$ext_dir_canonicalized/extension",
"extension is installed correctly on pg_available_extension_versions");
$ret = $node->safe_psql('postgres',
"select * from pg_available_extensions where name = '$ext_name2'");
is( $ret,
- "test_custom_ext_paths_using_directory|1.0|1.0|Test
extension_control_path",
+ "test_custom_ext_paths_using_directory|1.0|1.0|Test
extension_control_path|$ext_dir_canonicalized/extension",
"extension is installed correctly on pg_available_extensions");
$ret = $node->safe_psql('postgres',
"select * from pg_available_extension_versions where name =
'$ext_name2'"
);
is( $ret,
- "test_custom_ext_paths_using_directory|1.0|t|t|f|t|||Test
extension_control_path",
+ "test_custom_ext_paths_using_directory|1.0|t|t|f|t|||Test
extension_control_path|$ext_dir_canonicalized/extension",
"extension is installed correctly on pg_available_extension_versions");
+# Test that a non-superuser can not read the extension location on
+# pg_available_extensions
+$ret = $node->safe_psql('postgres',
+ "select location from pg_available_extensions where name =
'$ext_name2'",
+ connstr => "user=$user");
+is( $ret,
+ "<insufficient privilege>",
+ "extension location is hide on pg_available_extensions for insufficient
privilege");
+
+# Test that a non-superuser can not read the extension location on
+# pg_available_extension_versions
+$ret = $node->safe_psql('postgres',
+ "select location from pg_available_extension_versions where name =
'$ext_name2'",
+ connstr => "user=$user");
+is( $ret,
+ "<insufficient privilege>",
+ "extension location is hide on pg_available_extension_versions for
insufficient privilege");
+
# Ensure that extensions installed on $system is still visible when using with
# custom extension control path.
$ret = $node->safe_psql('postgres',
@@ -76,7 +102,6 @@ $ret = $node->safe_psql('postgres',
is($ret, "t",
"\$system extension is installed correctly on pg_available_extensions");
-
$ret = $node->safe_psql('postgres',
"set extension_control_path = ''; select count(*) > 0 as ok from
pg_available_extensions where name = 'plpgsql'"
);
diff --git a/src/test/regress/expected/rules.out
b/src/test/regress/expected/rules.out
index 77e25ca029e..383ac569c43 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1310,14 +1310,16 @@ pg_available_extension_versions| SELECT e.name,
e.relocatable,
e.schema,
e.requires,
- e.comment
- FROM (pg_available_extension_versions() e(name, version, superuser,
trusted, relocatable, schema, requires, comment)
+ e.comment,
+ e.location
+ FROM (pg_available_extension_versions() e(name, version, superuser,
trusted, relocatable, schema, requires, comment, location)
LEFT JOIN pg_extension x ON (((e.name = x.extname) AND (e.version =
x.extversion))));
pg_available_extensions| SELECT e.name,
e.default_version,
x.extversion AS installed_version,
- e.comment
- FROM (pg_available_extensions() e(name, default_version, comment)
+ e.comment,
+ e.location
+ FROM (pg_available_extensions() e(name, default_version, comment, location)
LEFT JOIN pg_extension x ON ((e.name = x.extname)));
pg_backend_memory_contexts| SELECT name,
ident,
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ac2da4c98cf..a6e39ade3d2 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -789,6 +789,7 @@ ExtensibleNodeEntry
ExtensibleNodeMethods
ExtensionControlFile
ExtensionInfo
+ExtensionLocation
ExtensionVersionInfo
FDWCollateState
FD_SET
@@ -1568,7 +1569,6 @@ LoadStmt
LocalBufferLookupEnt
LocalPgBackendStatus
LocalTransactionId
-Location
LocationIndex
LocationLen
LockAcquireResult
--
2.51.0
