Re: Bug in pg_dump

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