Re: Bug in pg_dump

From: Gilles Darold <gilles(dot)darold(at)dalibo(dot)com>
To: Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Bug in pg_dump
Date: 2015-01-16 13:43:45
Message-ID: 54B91591.2000505@dalibo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 16/01/2015 01:06, Jim Nasby wrote:
> On 1/15/15 5:26 AM, Gilles Darold wrote:
>> Hello,
>>
>> There's a long pending issue with pg_dump and extensions that have
>> table members with foreign keys. This was previously reported in this
>> thread
>> http://www.postgresql.org/message-id/CA+TgmoYVZkAdMGh_8EL7UVM472GerU0b4pnNFjQYe6ss1K9wDQ@mail.gmail.com
>> and discuss by Robert. All PostgreSQL users that use the PostGis
>> extension postgis_topology are facing the issue because the two
>> members tables (topology and layer) are linked by foreign keys.
>>
>> If you dump a database with this extension and try to import it you
>> will experience this error:
>>
>> pg_restore: [archiver (db)] Error while PROCESSING TOC:
>> pg_restore: [archiver (db)] Error from TOC entry 3345; 0
>> 157059176 TABLE DATA layer gilles
>> pg_restore: [archiver (db)] COPY failed for table "layer": ERROR:
>> insert or update on table "layer" violates foreign key constraint
>> "layer_topology_id_fkey"
>> DETAIL: Key (topology_id)=(1) is not present in table "topology".
>> WARNING: errors ignored on restore: 1
>>
>>
>> The problem is that, whatever export type you choose (plain/custom
>> and full-export/data-only) the data of tables "topology" and "layer"
>> are always exported in alphabetic order. I think this is a bug
>> because outside extension, in data-only export, pg_dump is able to
>> find foreign keys dependency and dump table's data in the right order
>> but not with extension's members. Default is alphabetic order but
>> that should not be the case with extension's members because
>> constraints are recreated during the CREATE EXTENSION order. I hope I
>> am clear enough.
>>
>> Here we have three solutions:
>>
>> 1/ Inform developers of extensions to take care to alphabetical
>> order when they have member tables using foreign keys.
>> 2/ Inform DBAs that they have to restore the failing table
>> independently. The use case above can be resumed using the following
>> command:
>>
>> pg_restore -h localhost -n topology -t layer -Fc -d
>> testdb_empty testdump.dump
>>
>> 3/ Inform DBAs that they have to restore the schema first then
>> the data only using --disable-triggers
>
> I don't like 1-3, and I doubt anyone else does...
>
>> 4/ Patch pg_dump to solve this issue.
>
> 5. Disable FK's during load.
> This is really a bigger item than just extensions. It would have the
> nice benefit of doing a wholesale FK validation instead of firing
> per-row triggers, but it would leave the database in a weird state if
> a restore failed...

I think this is an other problem. Here we just need to apply to
extension's members tables the same work than to normal tables. I guess
this is what this patch try to solve.

>
>> I attach a patch that solves the issue in pg_dump, let me know if it
>> might be included in Commit Fest or if the three other solutions are
>> a better choice. I also join a sample extension (test_fk_in_ext) to
>> be able to reproduce the issue and test the patch. Note that it might
>> exists a simpler solution than the one I used in this patch, if this
>> is the case please point me on the right way, I will be pleased to
>> rewrite and send an other patch.
>
> The only problem I see with this approach is circular FK's:
>
> decibel(at)decina(dot)local=# create table a(a_id serial primary key, b_id int);
> CREATE TABLE
> decibel(at)decina(dot)local=# create table b(b_id serial primary key, a_id
> int references a);
> CREATE TABLE
> decibel(at)decina(dot)local=# alter table a add foreign key(b_id) references b;
> ALTER TABLE
> decibel(at)decina(dot)local=#
>
> That's esoteric enough that I think it's OK not to directly support
> them, but pg_dump shouldn't puke on them (and really should throw a
> warning). Though it looks like it doesn't handle that in the data-only
> case anyway...

The patch is taking care or circular references and you will be warn if
pg_dump found it in the extension members. That was not the case before.
If you try do dump a database with the postgis extension you will be
warn about FK defined on the edge_data table.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2015-01-16 14:01:06 Re: PATCH: Reducing lock strength of trigger and foreign key DDL
Previous Message Andres Freund 2015-01-16 13:07:41 Re: Turning recovery.conf into GUCs