Re: Extensions support for pg_dump, patch v27

From: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>, Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>, PostgreSQL-Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Extensions support for pg_dump, patch v27
Date: 2011-02-08 10:51:11
Message-ID: 87d3n2ztnk.fsf@2ndQuadrant.fr
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Attached is an updated patch that incorporates all of the review I've

And that looks great, thanks. I've only had time to read the patch,
will play with it later on today, hopefully.

I've spotted a comment that I think you missed updating. The schema
given in the control file is now created in all cases rather than only
when the extension is not relocatable, right?

+ else if (control->schema != NULL)
+ {
+ /*
+ * The extension is not relocatable and the author gave us a schema
+ * for it. We create the schema here if it does not already exist.
+ */

I also note that the attached version doesn't contain \dX, is that an
happenstance or a choice you did here?

> done so far on the core code. This omits the contrib changes, which
> I've not looked at in any detail, and the docs changes since I've not
> yet updated the docs to match today's code changes. User-visible
> changes are that WITH NO DATA is gone, and instead there's
> pg_extension_config_dump(regclass, text) as per today's discussion.
> The latter is only lightly tested but seems to work as intended.

I think I will have to test it and get my head around it, but as we
said, it's a good change (simpler, only target user tables).

> I believe the core code is now in pretty good shape; the only open issue
> I have at the moment is that pg_dump will fail to suppress dumping of
> USER MAPPING objects that are in an extension. Which is something I'm
> not too excited about anyway. (The reason that's an issue is that I
> reverted most of the changes in pg_dump.c in favor of using pg_dump's
> already existing dependency mechanisms to suppress dumping unwanted
> items. The USER MAPPING code tries to bypass the DumpableObject
> representation, which means it doesn't get filtered.)

Well the pg_dump support code is very different than the one I did, so I
will have to learn about the dependency management there before I can
comment.

> The documentation still needs a good bit of work, but I hope to have
> this committed within a day or so.

Great! Again, thanks a lot, I like the way you simplified and cleaned
the patch, and I still recognise the code :)

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 Shigeru HANADA 2011-02-08 11:07:09 Re: review: FDW API
Previous Message Thom Brown 2011-02-08 10:12:47 Re: [HACKERS] Issues with generate_series using integer boundaries