Re: pg_restore COPY error handling

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> >> This is not super surprising because the original design approach for
> >> pg_restore was "bomb out on any sort of difficulty whatsoever". That
> >> was justly complained about, and now it will try to keep going after
> >> SQL-command errors ... but it sounds like the COPY-data processing
> >> part didn't get fixed properly.
>
> > Exactly so. Possibly because it appears to be non-trivial to detect
> > when it was a *COPY* command which failed (to differentiate it from some
> > other command). What I did was check for '<whitespace>COPY'. I'd be
> > happy to change that if anyone has a better suggestion though.
>
> ISTM you should be depending on the archive structure: at some level,
> at least, pg_restore knows darn well whether it is dealing with table
> data or SQL commands.

Right, it does higher up in the code but I don't believe that knowledge
is passed down to the level it needs to be because it wasn't necessary
before and the module API didn't include a way to pass that information
into a given module. I expect this is why the code currently depends on
libpq to tell it when it has entered a 'copy in' state. I'll look into
this though and see just how invasive it would be to get the information
to that level.

> Also, it might be possible to depend on whether libpq has entered the
> "copy in" state. I'm not sure this works very well, because if there's
> an error in the COPY command itself (not in the following data) then we
> probably don't ever get into "copy in" state.

This is what the code currently depends on, and libpq (properly, imv)
doesn't enter the 'copy in' state when the COPY command itself fails.
That's exactly how we end up in this situation where pg_restore thinks
it's looking at a SQL command (because libpq said it wasn't in a COPY
state) when it's really getting COPY data from the higher level in the
code.

Thanks,

Stephen