Re: extension_control_path: ignore empty elements and deduplicate directories

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: surya poondla <suryapoondla4(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Prashant Chinnam <crprashant(at)hotmail(dot)com>
Subject: Re: extension_control_path: ignore empty elements and deduplicate directories
Date: 2026-11-09 08:42:01
Message-ID: 254BF542-CF91-4468-B4B0-B0D278389E62@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Nov 8, 2026, at 12:37, surya poondla <suryapoondla4(at)gmail(dot)com> wrote:
>
> Hi Chao,
>
> I tested v1 on 0607aa61090, and did some extra code reading for this. A few comments:

Thank you very much for the review.

>
> 1. The new DEBUG3 message exposes the expanded paths to non-superusers. Since f3c9e341cdf commit, pg_available_extensions shows <insufficient privilege> as the location to them, and since your db5ed03217b commit it shows $system rather than the share path even to superusers. The GUC is also GUC_SUPERUSER_ONLY. But any role can SET client_min_messages = debug3 and see every directory, including the real share path. As an ordinary role (without pg_read_all_settings):
>
> SHOW extension_control_path;
> ERROR: permission denied to examine "extension_control_path"
>
> SET client_min_messages = debug3;
> SELECT count(*) FROM pg_available_extensions;
> DEBUG: get_extension_control_directories: path "/opt/ext/extension"
> DEBUG: get_extension_control_directories: path "/usr/local/pgsql/share/extension"
>
> find_in_path() has a similar DEBUG3 for dynamic_library_path, but non-superusers rarely reach it, whereas here any query on the views does. I would drop the elog.

Ah, good catch. I actually reviewed f3c9e341cdf, but I had completely forgotten about that patch.

>
> 2. In the original extension_control_path commit (4f7f7b03758), CREATE EXTENSION searched with find_in_path(), which errors out when the search reaches an empty element ("zero-length component in parameter", or "is not an absolute path" for a trailing one). Commit 81eaaa2c41d replaced that with find_in_paths(), which kept only the absolute-path check, and the same commit's "%s/extension" suffix is what turns an empty element into /extension. Rather than skipping silently, I would rather report the mistake, at least on master. The check would have to go in get_extension_control_directories() (or a check hook), since find_in_paths() only sees the resulting "/extension" and can't tell it from "/". That would also make the views and CREATE EXTENSION agree, which they haven't since 4f7f7b03758; otherwise ':' silently lists nothing while '' means $system. It is a behaviour change, though, the views, and extension_file_exists() (used for the CREATE FUNCTION unknown-language hint), would then error out on such a setting, so a silent skip may be the better fit for back branches.
>

I’m okay with raising an error for empty path elements. That would let us remove the DEBUG3 logging and resolve point 1 as well. I added the logging mainly for testing purposes.

My feeling is that this patch might not need to be back-patched.

> 3. As superuser, a relative element is shown by pg_available_extensions (resolved against the data directory), but CREATE EXTENSION rejects it in find_in_paths() once the search reaches it:
>
> SET extension_control_path = '$system:rel';
> SELECT name, location FROM pg_available_extensions WHERE name = 'relext';
> name | location
> --------+---------------
> relext | rel/extension
>
> CREATE EXTENSION relext;
> ERROR: component in parameter "extension_control_path" is not an absolute path
>
> The same place as in (2) could reject both.
>

Thanks for reporting that. I hadn’t noticed the problem. As this is a separate issue, I put the fix in 0002.

> 4. I'm not convinced 0002 is needed. The saving is one readdir() per repeated element in the views, for a setting I don't expect to be common. The location checks in the new tests already pass on master, because the views deduplicate by extension name.
>

As I mentioned in the original patch email, this change is of relatively low value, but I think it’s also harmless. For example, if an extension directory is on network storage, avoiding a duplicate directory scan would be preferable. So I’d leave it as 0003 for now and wait for more opinions.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Attachment Content-Type Size
v2-0001-Reject-empty-elements-in-extension_control_path.patch application/octet-stream 3.7 KB
v2-0002-Reject-relative-paths-consistently-in-extension_c.patch application/octet-stream 3.3 KB
v2-0003-Deduplicate-extension-control-search-directories.patch application/octet-stream 5.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Zsolt Parragi 2026-11-09 08:46:01 Re: BGWORKER_INTERRUPTIBLE workers are never restarted after DROP/ALTER/CREATE DATABASE
Previous Message Michael Paquier 2026-11-09 08:10:23 Re: Reject access to unlogged tables in contrib modules