| From: | surya poondla <suryapoondla4(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(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-08 04:37:54 |
| Message-ID: | CAOVWO5oxLQMasge2qKiMj==BdYLc8mC4sDS5ngM-XkdvDugcPA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Chao,
I tested v1 on 0607aa61090, and did some extra code reading for this. A few
comments:
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.
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.
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.
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.
Regards,
Surya Poondla
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ritesh Kumar | 2026-11-08 05:28:09 | Re: Fix recovery conflict canceling the startup progress timeout |
| Previous Message | 신성준 | 2026-11-08 04:27:49 | Re: Add wait events for server logging destination writes |