Re: Extensions support for pg_dump, patch v27

On Thu, Jan 27, 2011 at 22:48, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
> Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com> writes:
>> I found pg_restore with -c option fails when an extension is created
>> in pg_catalog.
> Nice catch, thank you very much (again) for finding those :)

Seems good.

>>   extern bool extension_relocatable_p(Oid ext_oid);
> predicate.  Maybe I've done too much Emacs Lisp coding at the time I
> added that function, but it looked natural (enough) to me :)

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.

* 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.

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

* 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.)

* 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".

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

--
Itagaki Takahiro