Thread: [PATCH] pg_restore COPY error handling

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> >   It seems like pg_restore really should be able to handle COPY errors
> >   correctly by skipping to the end of the COPY data segment when the
> >   initial COPY command comes back as an error.
>
> Send a patch ;-)

This is what I get for knowing how to copy & paste C code, eh? ;-)

Attached is a patch to pg_restore, against HEAD but I think it'd work
against 8.1 just fine, to better handle it when a COPY command fails
(for whatever reason) during a DB restore.

Attempting to restore from a dump with 2 table objects under 8.1:

------------------------
sfrost@snowman:/data/sfrost/postgres> pg_restore -d tsf -Fc tiger_test.dump
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 412; 16672 1135494071 SCHEMA tiger_test postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  permission denied for database tsf
    Command was:
CREATE SCHEMA tiger_test;
pg_restore: [archiver (db)] could not execute query: ERROR:  schema "tiger_test" does not exist
    Command was: ALTER SCHEMA tiger_test OWNER TO postgres;
pg_restore: [archiver] Error from TOC entry 21177; 1259 1135494072 TABLE bg01_d00 postgres
pg_restore: [archiver] could not set search_path to "tiger_test": ERROR:  schema "tiger_test" does not exist
pg_restore: [archiver (db)] could not execute query: ERROR:  type "public.geometry" does not exist
    Command was: CREATE TABLE bg01_d00 (
    ogc_fid integer,
    wkb_geometry public.geometry,
    area numeric(20,5),
    perimeter numeric...
pg_restore: [archiver (db)] could not execute query: ERROR:  schema "tiger_test" does not exist
    Command was: ALTER TABLE tiger_test.bg01_d00 OWNER TO postgres;
pg_restore: [archiver (db)] Error from TOC entry 21178; 1259 1135497928 TABLE bg02_d00 postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  type "public.geometry" does not exist
    Command was: CREATE TABLE bg02_d00 (
    ogc_fid integer,
    wkb_geometry public.geometry,
    area numeric(20,5),
    perimeter numeric...
pg_restore: [archiver (db)] could not execute query: ERROR:  schema "tiger_test" does not exist
    Command was: ALTER TABLE tiger_test.bg02_d00 OWNER TO postgres;
pg_restore: [archiver (db)] Error from TOC entry 21609; 0 1135494072 TABLE DATA bg01_d00 postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  relation "bg01_d00" does not exist
    Command was: COPY bg01_d00 (ogc_fid, wkb_geometry, area, perimeter, bg01_d00_, bg01_d00_i, state, county, tract,
blkgroup,name, lsad, ls... 
pg_restore: [archiver (db)] Error from TOC entry 21610; 0 1135497928 TABLE DATA bg02_d00 postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  syntax error at or near "1" at character 1
    Command was: 1
0103000000010000007D000000F1283A3752FB55C0E38C825CB98041403BC3D4963AFB55C0D8D30E7F4D80414015376E313FFB55C07AE40F069E7F4140...
WARNING: errors ignored on restore: 9
------------------------

As you can see, it's treating the data (the 01030000.... bit) as a
command, which is most certainly not right, especially when it *knows*
that the COPY command failed.

Attempting to restore from a dump with 2 table objects with patch:

------------------------
sfrost@snowman:/data/sfrost/postgres/testinstall> bin/pg_restore -d tsf -Fc -h localhost ../tiger_test.dump
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 412; 16672 1135494071 SCHEMA tiger_test postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  role "postgres" does not exist
    Command was: ALTER SCHEMA tiger_test OWNER TO postgres;
pg_restore: [archiver (db)] Error from TOC entry 21177; 1259 1135494072 TABLE bg01_d00 postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  type "public.geometry" does not exist
    Command was: CREATE TABLE bg01_d00 (
    ogc_fid integer,
    wkb_geometry public.geometry,
    area numeric(20,5),
    perimeter numeric...
pg_restore: [archiver (db)] could not execute query: ERROR:  relation "tiger_test.bg01_d00" does not exist
    Command was: ALTER TABLE tiger_test.bg01_d00 OWNER TO postgres;
pg_restore: [archiver (db)] Error from TOC entry 21178; 1259 1135497928 TABLE bg02_d00 postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  type "public.geometry" does not exist
    Command was: CREATE TABLE bg02_d00 (
    ogc_fid integer,
    wkb_geometry public.geometry,
    area numeric(20,5),
    perimeter numeric...
pg_restore: [archiver (db)] could not execute query: ERROR:  relation "tiger_test.bg02_d00" does not exist
    Command was: ALTER TABLE tiger_test.bg02_d00 OWNER TO postgres;
pg_restore: [archiver (db)] Error from TOC entry 21609; 0 1135494072 TABLE DATA bg01_d00 postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  relation "bg01_d00" does not exist
    Command was: COPY bg01_d00 (ogc_fid, wkb_geometry, area, perimeter, bg01_d00_, bg01_d00_i, state, county, tract,
blkgroup,name, lsad, ls... 
pg_restore: [archiver (db)] Error from TOC entry 21610; 0 1135497928 TABLE DATA bg02_d00 postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  relation "bg02_d00" does not exist
    Command was:

COPY bg02_d00 (ogc_fid, wkb_geometry, area, perimeter, bg02_d00_, bg02_d00_i, state, county, tract, blkgroup, name,
lsad,... 
WARNING: errors ignored on restore: 7
------------------------

Here it correctly handles that the COPY command failed and just skips
past the data portion of the COPY.  This lets it see the second object
properly (which we expect to fail) so that it can attempt to load it.
For a small case like this it's meaningless (this was just my test
case), for very large databases, being able to make it past errors
like these is essential...

    Thanks!

        Stephen

* Stephen Frost (sfrost@snowman.net) wrote:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> > Stephen Frost <sfrost@snowman.net> writes:
> > >   It seems like pg_restore really should be able to handle COPY errors
> > >   correctly by skipping to the end of the COPY data segment when the
> > >   initial COPY command comes back as an error.
> >
> > Send a patch ;-)
>
> This is what I get for knowing how to copy & paste C code, eh? ;-)
>
> Attached is a patch to pg_restore, against HEAD but I think it'd work
> against 8.1 just fine, to better handle it when a COPY command fails
> (for whatever reason) during a DB restore.
[...]
>     Command was:
>
> COPY bg02_d00 (ogc_fid, wkb_geometry, area, perimeter, bg02_d00_, bg02_d00_i, state, county, tract, blkgroup, name,
lsad,... 
> WARNING: errors ignored on restore: 7
> ------------------------

Of course, looking at this again, I'm afraid my COPY-attempt-detection
logic isn't quite enough.  I was hoping it'd be taken care of by
_sendSQLLine, but apparently not.

The line:
if (strncasecmp(qry->data,"COPY ",5) == 0) AH->pgCopyIn = -1;

Needs to be changed to handle whitespace in front of the actual 'COPY',
unless someone else has a better idea.  This should be reasonably
trivial to do though...  If you'd like me to make that change and send
in a new patch, just let me know.

    Thanks,

        Stephen