> On Nov 11, 2026, at 03:27, Matheus Alcantara <[email protected]> wrote: > > Thanks for the new version! > > I tested v3 and all the previous comments are addressed. CREATE FUNCTION > and DO with an unknown language now report the original error with > '$system:', ':' and '$system:rel', and the hint is still shown when a > remaining directory contains the extension. > > Regarding the backpatch, I don't think that having a different behaviour > on master and on the back branches is a good idea. If '$system:' raises > an error on 20 but is silently ignored on 18 and 19, the same setting > behaves differently depending on the version, which seems confusing to > me, especially for a parameter that was only introduced on 18. I think > that we should either raise the error on all branches or silently ignore > the empty elements on all branches, and then backpatch whichever we > choose. (CCing Peter so we can get a committer thought on this). > > This is also related to the "directory" documentation thread [1] that > you previously mention, where the '$system/foo' case came up. > Mahendrakar noticed there that with '$system/foo:$system' > pg_available_extensions lists the extensions from $system, while CREATE > EXTENSION fails on any of them, and asked whether a check hook to reject > such elements at SET time would be worth pursuing. With v3-0002 both now > fail consistently: > > SET extension_control_path = '$system/foo:$system'; > SELECT count(*) FROM pg_available_extensions; > ERROR: component in parameter "extension_control_path" is not an absolute > path > > So the inconsistency seems to be fixed here. But his concern about the > check hook also applies to 0002: a value like '$system:$system/foo' > works today for any extension found in $system, and with 0002 it fails > on every extension lookup. The only difference with a check hook would > be that the error shows up when the value is set (or when the server > starts) instead of when the path is used. So I think that whatever we > decide for the backpatch should also cover 0002. > > Some small comments on v3: > > 1: On 0002, the location is allocated before the absolute path check, > so the skip branch needs to free it again. Since location->macro is > only set for $system, which always expands to an absolute path, the > pfree(location->macro) can't be reached. Maybe we could build and check > the mangled path first and only allocate the location when the element > is kept? >
Fixed in v4 > 2: The USE_ASSERT_CHECKING block on find_in_paths() seems a bit too > much to me. Now that it's static and only used with the output of > get_extension_control_directories(), I think that > Assert(is_absolute_path(path)) and the header comment are enough. > Removed that in v4. > 3: The 0002 commit message still only mentions replacing the check in > find_in_paths() with an assertion. It would be good to also mention > that find_in_paths() is now static, that system_dir is canonicalized, > and that skip_invalid_path now also skips relative elements. Updated 0002’s commit message in v4. > > [1] > https://postgr.es/m/cabkiuwpgilzdg4d8zrbj6dhnwyghqkfxz2gdstxgeodzh6b...@mail.gmail.com > > -- > Matheus Alcantara > EDB: https://www.enterprisedb.com PFA v4: * 0001 - unchanged from 0003 * 0002 - addressed Matheus’ review comments Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
v4-0001-Reject-empty-elements-in-extension_control_path.patch
Description: Binary data
v4-0002-Reject-relative-paths-consistently-in-extension_c.patch
Description: Binary data
