Re: Extensions support for pg_dump, patch v27

Itagaki Takahiro <itagaki.takahiro@gmail.com> writes:
> Hmmm, I like extension_is_relocatable() or so...
> Including the above, I wrote a patch on your patch for minor
> cleanup. Please merge reasonable parts in it.

After review, I included all your proposed changes, thanks a lot!
Please find attached a new version of the patch, v29, including your
changes and merged against HEAD from this morning (there was no
conflict, but still).

> * access() is not portable.
> The pre-checking with access() doesn't seems needed because
> the same error will be raised in parse_extension_control_file().
>
> * There are some dead code in the patch.
> For example, you exported ObjectAddresses to public, but it is not
> used in extension.c actually. I reverted some of changes.

Thanks, those are due to late refactoring, removal of features and
adjustments etc.  Doing that in "free time" rather than full time these
busy days, so I've missed some cleaning.

> * Should we support absolute control file paths?
> Absolute paths are supported by get_extension_absolute_path(),
> but I'm not sure actual use-cases.

Well I don't see no harm in allowing non-core compliant extension
packaging, as this feature is not targeting only BSD compatible Open
Source extensions such as contribs.  It seems to me to be a case of
mechanism versus policy, I don't think our policy here should mean that
we alter the mechanism for others.

> * Each ereport(ERROR) should have a reasonable errcode unless
> they are an internal logic error, and whether the error message
> follows our guidline (starting with a lower case character, etc.)

Done.

> * Changed create_extension_with_user_data to a static variable.
> CreateExtensionAddress and create_extension are still exported.
> We could have better names for them -- CurrentExtensionAddress
> and in_create_extension?  Or, in_create_extension might be
> replaced with "CreateExtensionAddress.objectId != InvalidOid".

Well there has been enough discussion about those names I think, current
one where voted by Alvaro and Robert IIRC.  I'm open to changes, but
would now prefer to include that in the commiter's work :)

> * Added psql tab completion for CREATE/DROP/ALTER EXTENSION.
> * Use palloc0() instead of palloc() and memset(0).
> * Several code cleanup.

Thanks a lot!

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