Re: Extensions, this time with a patch

From: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>, David E(dot) Wheeler <david(at)kineticode(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Extensions, this time with a patch
Date: 2010-10-22 15:25:07
Message-ID: m2ocam8d2k.fsf@2ndQuadrant.fr
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>> Fixed by having the CREATE EXTENSION command consider it's being given
>> the name of the extension as found in the control file, rather than the
>> control file base name.
>
> Hang on. Did I miss something? Why doesn't the control file name equal
> the extension name? I think the idea that you have to scan the whole
> share directory and parse all control files to find the one you need is
> a bit strange.

The default behavior is to have the control file name the same as the
extension's file name, so there's no directory scanning in this case. It
so happen that the existing "extensions" are not currently following the
rule, even when we limit ourselves to contrib, so I adjusted the code to
be able to be more friendly.

Performance wise, it only concerns the command CREATE EXTENSION and the
queries to the pg_extensions system view, that's used in \dx. Does not
seem that critical.

Now, if we want to do it the other way round and force extension name to
be the filename, we will have to live with all the restrictions that
filename imposes and that are not the same depending on the OS and the
filesystem, I think, and with systems where we have no way to know what
is the filesystem encoding. Am I wring in thinking that this might be a
problem?

If that's not a problem and we want to have the extension name into the
control file name, then I'll go remove the name property there.

> It seems more sensible to allow the variation to occur in the Makefile,
> i.e. _int/Makefile should contain
> EXTENSION=intarray
>
> Was this discussed previously? If so, why was this idea rejected?

It's already possible to do this. But the contrib uses _int.sql.in and
MODULE_big = _int and I didn't change that. I had the control file named
the same as the script file, but with the current implementation we can
change that (there's a script property in the control file).

>> > * pg_execute_from_file() and encoding
>> > It expects the file is in server encoding, but it is not always true
>> > because we support multiple encodings in the same installation.
>> > How about adding encoding parameter to the function?
>> > => pg_execute_from_file( file, encoding )
>
> This seems sensible ... at least as sensible as it is to allow COPY to
> specify the encoding.

Well why not, for convenience, but you can surround the call to this
function with SET client_encoding and get the same behaviour. Will add
the argument if required, though.

>> > CREATE EXTENSION could have optional ENCODING option.
>> > => CREATE EXTENSION name [ ENCODING 'which' ]
>
> This seems like putting the burden on the user on getting the command
> right. That seems prone to failure.

Given that the control file now supports an encoding parameter, I don't
see what this is good for, but I might be missing something obvious for
people working with different encodings etc. Japan seems to be quite
special as far as encodings are concerned.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dimitri Fontaine 2010-10-22 15:32:34 Re: Extensions, this time with a patch
Previous Message Tom Lane 2010-10-22 15:19:48 Re: crash in plancache with subtransactions