Re: Extensions, this time with a patch

Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:
> Open Items :
>
>  - cfparser

Still in attached v1 patch, but will repost separately, as proposed by
Álvaro.

>  - User Documentation. Where in the manual do I write it?

Chapter 35. Extending SQL looked like a good choice, there it is. Needs
to get expanded with latest additions.

>  - Naming of the "control" file,
>  <extension>.{control,registry,install}

Issue still to discuss. I'm happy with the current .control name.

>  - Handling of custom_variable_classes

This version of the patch address the point. The problem I wanted to
solve is that 'create extension' has no chance to edit the user
configuration, such as custom_variable_classes. Also, in the debian
world it is a policy violation for a package to modify another package's
configuration file.

So I wanted the extension mechanism to be able to append some classes to
the custom_variable_classes without touching the configuration
file. That's why pg_extension has this custom_class column.

The v1 patch, attached, rework this GUC so that it's SUSET. Invalid
variables are now kept aside while processing the configuration, so that
it's possible to scan them again when custom_variable_classes changes,
and to make them available.

Based on this change, postinit now will scan the pg_extension catalog
for the connected database and add all classes defined there. So that if
an extension register its own class, and the user defines some variables
in the configuration file, it's not necessary for him to edit the global
custom_variable_classes any more.

Also, at CREATE EXTENSION time, should an extension specific classes be
given, custom_variable_classes is changed on the fly, and default values
from the control file can get loaded too.

Current code is to be read as a proposal, open to discussion of course:
it seemed to me preferable to write it (having not heard back from my
earlier mail) as an opening.

>  - User data tables
>
>    An extension can install plain relations, and that's fine. The
>    problem is when the data in there are changed after installing.
>    Because the name of the game here is to exclude the table from the
>    dumps, of course the data will not be in there.
>
>    The solution would be to offer extension's author a way to 'flag'
>    some tables as worthy of dumping, I think marking the dependency as
>    DEPENDENCY_NORMAL rather then DEPENDENCY_INTERNAL will do the trick:
>
>      SELECT pg_extension_flag_dump(oid);

That's in the attached patch too.

>  - Extension Upgrading

That was a mistake to raise this subject this early, it seems wise to
defer that to a later patch. Much remains to be designed here.

>  - Bugs to fix
>
>    There's at least one where drop extension leaves things behind,
>    although it uses performDeletion(). The dependencies are fine enough
>    so that the leftover objects are not part of the dump done before to
>    drop, though.

That well seems to me to be an existing bug that I'm just putting in the
light. The reading of comments from findDependentObjects() makes me
think that following nested DEPENDENCY_INTERNAL levels is broken, but I
didn't have time to figure out how exactly, nor to try to fix.

So, v1 patch attached, and repository available too:

  http://git.postgresql.org/gitweb?p=postgresql-extension.git;a=shortlog;h=refs/heads/extension

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