Re: Bug in pg_dump

On Tue, Feb 24, 2015 at 2:17 AM, Gilles Darold <gilles.darold@dalibo.com> wrote:

Looks great to me, I have tested with the postgis_topology extension
everything works fine.

Actually, after looking more in depth at the internals of pg_dump I think that both patches are wrong (did that yesterday night for another patch). First this patch marks a table in an extension as always dumpable:
+      /* Mark member table as dumpable */
+      configtbl->dobj.dump = true;

And then many checks on ext_member are added in many code paths to ensure that objects in extensions have their definition never dumped.

But actually this assumption is not true all the time per this code in getExtensionMemberShip:
        if (!dopt->binary_upgrade)
            dobj->dump = false;
        else
            dobj->dump = refdobj->dump;

So this patch would break binary upgrade where some extension objects should be dumped (one reason why I haven't noticed that before is that pg_upgrade tests do not include extensions).

Hence, one idea coming to my mind to fix the problem would be to add some extra processing directly in getExtensionMembership() after building the objects DO_TABLE_DATA with makeTableDataInfo() by checking the FK dependencies and add a dependency link with addObjectDependency. The good part with that is that even user tables that reference extension tables with a FK can be restored correctly because their constraint is added *after* loading the data. I noticed as well that with this patch the --data-only mode was dumping tables in the correct order.

Speaking of which, patches implementing this idea are attached. The module test case has been improved as well with a check using a table not in an extension linked with a FK to a table in an extension.

--

Michael

-- 
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org