public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andrew Dunstan <[email protected]>
To: Matheus Alcantara <[email protected]>
To: Chao Li <[email protected]>
Cc: Rohit Prasad <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: Include extension path on pg_available_extensions
Date: Thu, 1 Jan 2026 12:19:35 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<176282677490.2081919.14769189141245328542.pgcf@coridan.postgresql.org>
	<[email protected]>
	<[email protected]>
	<[email protected]>

 >

On 2025-11-13 Th 9:36 AM, Matheus Alcantara wrote:
> On Thu Nov 13, 2025 at 6:11 AM -03, Chao Li wrote:
>>> <v5-0001-Add-path-of-extension-on-pg_available_extensions.patch>
>> 1 - commit comment
>> ```
>> 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.
>> ```
>>
>> First, there is a typo: "for users that has” should be “have”.
>>
>> Then, Is this still true? The code change shows:
>> ```
>> +	/* We only want to show extension paths for superusers. */
>> +	if (superuser_arg(GetUserId()))
>> +	{
>> ```
>>
>> And the code change says:
>> ```
>> +       GUC. Only superusers can see the contents of this column.
>> ```
>>
>> But the TAP test contains a case for normal user:
>> ```
>> +# 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|$ext_dir_canonicalized/extension|Test extension_control_path",
>>   	"extension is installed correctly on pg_available_extensions”);
>> ```
>>
>> And I tried to run the TAP test, it passed.
>>
>> So I’m really confused here.
>>
> Oops, I forgot to update the commit message. The extension location is
> only visible for super users. Fixed on v6.
>
> The created user on TAP test is used to check that non superusers can not
> see the location column content. The other test case changes are using
> the 'postgres' user, so it should see the extension location value.
>
>> 2
>> ```
>> +	else
>> +		return "<insufficient privilege>";
>> +}
>> ```
>>
>> Why do we just show nothing (empty string)? Every row showing a long string of <insufficient privilege> just looks not good, that’s a lot of duplications.
>>
> This is the same behavior of pg_stat_activity. Returning an empty string
> could make the user think that something is not right with the
> implementation. Returning <insufficient privilege> for every row is a
> lot of duplication I agree but I think that it make clear for the user
> that it don't have the necessary role to view the column value.
>
> Thanks for reviewing!
>


Committed.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Include extension path on pg_available_extensions
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox