Hi, While reviewing patch [1], I noticed two issues with extension path population.
1. Empty path elements are silently interpreted as "/extension"
Say a user sets extension_control_path = '$system:', it will be interpreted as
two paths: $system and an empty string, and the empty string will be expanded
to "/extension". Since "/extension" is an absolute path, it will be searched
for extensions. If the host machine doesn't have "/extension", the path will be
silently ignored; but if the directory happens to exist, it will be searched
for extensions, which doesn't sound desirable, because the user might not be
aware of that at all.
Adding a trailing colon is an easy mistake to make, but silently searching
"/extension" is unintended behavior. So I think we should silently discard
empty path elements. An entirely empty string ('') is interpreted as $system as
documented, and that behavior should be retained.
2. It may read the same directory more than once
Say a user sets extension_control_path = '$system:$system', that's an obvious
duplicate path, but the current implementation will scan $system twice when
listing available extensions, and only deduplicate extension names later.
There is no correctness problem here, but deduplicating search paths may avoid
unnecessary directory reads. I think this is a relatively low-priority but
useful optimization.
I split the fixes into two commits, so that committers can handle them
separately.
I added DEBUG3 logging to print the paths to search, mainly so the tests can
verify them. It also allows users to view the expanded search paths.
[1]
https://postgr.es/m/cabkiuwpgilzdg4d8zrbj6dhnwyghqkfxz2gdstxgeodzh6b...@mail.gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
v1-0001-Ignore-empty-elements-in-extension_control_path.patch
Description: Binary data
v1-0002-Deduplicate-extension-control-search-directories.patch
Description: Binary data
