Re: Include extension path on pg_available_extensions - Mailing list pgsql-hackers

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/