Re: Extensions, this time with a patch

On Tue, Oct 19, 2010 at 9:33 PM, Dimitri Fontaine
<dimitri(at)2ndquadrant(dot)fr> wrote:
> Fixed in v4, attached.

It works as expected in basic functionality, so I pick up edge case issues.
Some of them might be a problem for the patch; They might come from our
non-standardized module design.

==== Source code ====
* It still has a warning.
backend/utils/init/postinit.c needs #include "commands/extension.h".

* It has duplicated oids.
$ ./duplicate_oids
3627
3695
3696
3697
3698
3699

==== Tests for each contrib module ====
* Some modules dumps noisy NOTICE or WARNING messages.
We need to fix btree_gist, chkpass, cube, hstore, isn, ltree, pg_trgm,
and seg. CREATE EXTENSION commands for them dumps the contents of installer
script as CONTEXT. We can add SET client_min_messages in each script, but
there is an issue to use SET command in scripts, discussed in below.

* Modules that should be added to shared_preload_libraries.
auto_explain, dummy_seclabel, passwordcheck, and pg_stat_statements are
designed to be preloaded with shared_preload_libraries. If we support
modifying GUC variables vis SQL, we could add some hooks to update conf files.

* Modules that only have *.sql, but not *.sql.in.
There are no *.control files for intagg and intarray, maybe because they
don't have *.sql.in. But we should support them, too.

* Hyphen (-) in module name
'uuid-ossp' has a hyphen in the name. Do we need double-quotes to install
such extensions? (CREATE EXTENSION "uuid-ossp" works.)
Also, custom_variable_classes doesn't support hyphens; only [A-Za-z0-9_]
are accepted for now, but will we also support hyphens?

* xml2 module has a different extension name from the directory name.
The directory name is 'xml2', but the extension name is 'pgxml'.
I'm not sure whether we should change the names. Or, merging xml2 module
into core and deprecating xml2 might be the best solution.

* spi module has multiple *.so, but only one *.control.
'spi' module generates multiple libraries: 'autoinc', 'insert_username',
'moddatetime', 'refint', and 'timetravel'. But it has only autoinc.control.
We might need control files for each library.

==== CREATE EXTENSION command ====
* Environment could be modified by the installer script.
=# SHOW search_path; => "$user",public
=# CREATE EXTENSION dblink;
=# SHOW search_path; => public
because almost all of the modules have SET search_path in the scripts:
-- Adjust this setting to control where the objects get created.
SET search_path = public;

Is is an intended behavior? Personally, I want the installer to run in sandbox.
One idea is to rewrite module scripts to use BEGIN - SET LOCAL - COMMIT,
but we cannot execute CREATE EXTENSION in transaction if do so.

* Non-ASCII characters in script
User-defined module could have non-ascii characters in their install script.
They often have "SET client_encoding" at the beginning, and it works as
expected if executed by psql. However, it raises encoding errors if executed
by CREATE EXTENSION because they are executed as one multi-command.

Are there any solution to solve the issue? I think it's a bit difficult
because encodings in database, script, and client might be different.
If encodings in script and client are different, the server might
need to handle two different client encodings in the same time.

--
Itagaki Takahiro