From: | "David E(dot) Wheeler" <david(at)justatheory(dot)com> |
---|---|
To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Gabriele Bartolini <gabriele(dot)bartolini(at)enterprisedb(dot)com>, Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
Subject: | Re: RFC: Additional Directory for Extensions |
Date: | 2025-02-03 20:42:43 |
Message-ID: | FD9A2896-9FB7-4DBA-AA0B-0D4C63892A25@justatheory.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Peter,
> prefix= should only be set when running the "install" target, not when building (make all).
I see. I confirm that works. Still, don’t all the other parameters work when passed to any/all targets? Should this one have an extension-specific name?
>> So I suspect the issue is that, when looking for SQL files, the patch needs to use the directory parameter[4] when it’s set --- and it can be an absolute path! Honestly I think there’s a case to be made for eliminating that parameter.
>
> Possibly. I didn't know why extensions would use that parameter, before you showed an example.
ISTM it does more harm than good. The location of extension files should be highly predictable. I think the search path functionality mitigates the need for this parameter, and it should be dropped.
>> I thought it would put the files into /Users/david/pgsql-test/extension, not /Users/david/pgsql-test/share/extension? I guess that makes sense; but then perhaps the search path should be for prefixes, in which case I’d use a config like:
>> ``` ini
>> extension_control_path = '/Users/david/pgsql-test:$system'
>> dynamic_library_path = '/Users/david/pgsql-test/lib:$libdir'
>> ```
>> And the search path would append `share/extension` to each path. But then it varies from the behavior of `dynamic_library_path`. :-(
>
> Yes exactly. This is meant to be symmetrical.
IOW, `extension_control_path`s should always end in `share/extension` and `dynamic_library_path`s should always end in `lib`, yes?
> We could have a setting that just sets the top-level prefixes to search and would thus supersede both of these settings. But then you introduce another lay of complications, such as, the subdirectory structure under the prefix is not necessarily fixed. It could be lib, lib/postgresql, lib64, etc., similar under share. It's not even required that there is a common prefix.
I would say that, under those prefixes, the directory names have to be defined by PostgreSQL, not by compile-time options. It seems to me that by providing search paths there is less of a need to monkey with directory names.
>>> I have solved this by just stripping off "$libdir/" from the front of the filename. This works for now. We can think about other ways to tweak this, perhaps, but I don't see any drawback to this in practice.
>>
>> I think this makes perfect sense. I presume it’s stripped out *before* replacing the MODULE_PATHNAME string in SQL files, yes?
>
> No, actually it's done after. Does it make a difference?
If you mean it strips it out at runtime, it just feels a little less clean to me than if it was formatted properly before `make install`ing.
>> Although the current path explicitly searches for control files, I’d like to suggest a more generic name, since the point is to find extensions; control files are just the (current) method for identifying them. I suggest `extension_path`. Although given the above, maybe it should be all about prefixes.
>
> If we implemented a prefix approach, then 'extension_path' could be a good name. But right now we're not.
I’m not sure what difference it makes, especially since each directory in the path is expected to end in `share/extension`, not `share/control`. Feels more symmetrical to me.
>> I like this idea, though I quibble with the term `$system`, which seems like it would identify SYSCONFDIR, not SHAREDIR/extension. How about `$extdir` or, if using prefixes, `$coreprefix` or something.
>
> I'm not attached to '$system', but I don't see how you get from that to SYSCONFDIR.
Because it’s the only pg_config item that includes the word “Sys” in it, meaning the operating system.
> The other suggestions also have various ways of misinterpreting them. We can keep thinking about this one.
I like $extdir. Short, clear, and not conflicting with existing pg_config options. I guess it could be misinterpreted as “extra directory” or something, but since there is no such thing it seems like a minor risk. I am likely overlooking something though.
>>> Note that the path elements should typically end in
>>> <literal>extension</literal> if the normal installation layouts are
>>> followed. (The value for <literal>$system</literal> already includes
>>> the <literal>extension</literal> suffix.)
>> In fact, if we’re using `prefix` as currently implemented, it should end in `share/extension`, no?
>
> It could be share/postgresql/extension, too. So I didn't want to overdocument this, because it could vary. The examples just above this are hopefully helpful.
Yeah, but if we can define it specifically, and disallow its modification, it simplifies things. And if understand correctly, paths like SHAREDIR defined at compile time are absolute paths, not suffixes to a prefix, yes?
But perhaps our packaging friends object to disallowing customization of this suffix?
Best,
David
From | Date | Subject | |
---|---|---|---|
Next Message | Jeff Davis | 2025-02-03 20:45:24 | injection points for hash aggregation |
Previous Message | Masahiko Sawada | 2025-02-03 20:36:21 | Re: pg_rewind with --write-recovery-conf option doesn't write dbname to primary_conninfo value. |