Thread

1. pg_restore direct to database is broken for --insert dumps

   Tom Lane <tgl@sss.pgh.pa.us> — 2012-01-04T18:13:02Z

   In http://archives.postgresql.org/pgsql-admin/2012-01/msg00008.php
   it's pointed out that recent versions of pg_restore fall over on
   archives made with -Fc --inserts (or --column-inserts), but only when
   restoring direct to database; if you ask for text output it's perfectly
   fine.  Investigation shows that the problem is that individual INSERT
   commands are being broken apart at arbitrary buffer boundaries --- you
   don't see any problem in text output, but when the bufferloads are
   submitted as separate PQexec calls, of course bad things happen.
   
   I believe this worked okay up until my patch here:
   http://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=6545a901a
   which removed the mini SQL lexer in pg_backup_db.c.  I had supposed that
   that had no useful function except to separate COPY data from
   not-COPY-data, but in reality it had another function of ensuring that
   INSERT commands split across zlib bufferload boundaries would get
   reassembled before they are submitted to PQexec.
   
   Not entirely sure what to do about this.  We could consider reverting
   the aforesaid patch and trying to find another way of fixing that code's
   failure to cope with standard-conforming strings, but I'm not sure that
   there's a good way to know whether standard_conforming_strings is active
   here, and anyway that code was ugly as sin and I'd rather not resurrect
   it.  But on the other hand, there are no clear line boundaries in the
   compressed data, and we can't introduce any without (a) worsening
   compression and (b) breaking compatibility with existing dump files.
   
   Anybody have any bright ideas?  I'm fresh out at the moment.
   
   			regards, tom lane
   
   

2. Re: pg_restore direct to database is broken for --insert dumps

   Andrew Dunstan <andrew@dunslane.net> — 2012-01-04T22:27:05Z

   
   On 01/04/2012 01:13 PM, Tom Lane wrote:
   > In http://archives.postgresql.org/pgsql-admin/2012-01/msg00008.php
   > it's pointed out that recent versions of pg_restore fall over on
   > archives made with -Fc --inserts (or --column-inserts), but only when
   > restoring direct to database; if you ask for text output it's perfectly
   > fine.  Investigation shows that the problem is that individual INSERT
   > commands are being broken apart at arbitrary buffer boundaries --- you
   > don't see any problem in text output, but when the bufferloads are
   > submitted as separate PQexec calls, of course bad things happen.
   >
   > I believe this worked okay up until my patch here:
   > http://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=6545a901a
   > which removed the mini SQL lexer in pg_backup_db.c.  I had supposed that
   > that had no useful function except to separate COPY data from
   > not-COPY-data, but in reality it had another function of ensuring that
   > INSERT commands split across zlib bufferload boundaries would get
   > reassembled before they are submitted to PQexec.
   >
   > Not entirely sure what to do about this.  We could consider reverting
   > the aforesaid patch and trying to find another way of fixing that code's
   > failure to cope with standard-conforming strings, but I'm not sure that
   > there's a good way to know whether standard_conforming_strings is active
   > here, and anyway that code was ugly as sin and I'd rather not resurrect
   > it.  But on the other hand, there are no clear line boundaries in the
   > compressed data, and we can't introduce any without (a) worsening
   > compression and (b) breaking compatibility with existing dump files.
   >
   > Anybody have any bright ideas?  I'm fresh out at the moment.
   >
   > 			
   
   
   I pondered this while out on my daily constitutional. The first thing 
   that occurred to me is that it would possibly have been better if we'd 
   made pg_dump not use a quoting mechanism whose behaviour is dependent on 
   a setting (e.g. E'' or dollar quoting). But I guess that's water under 
   the bridge.
   
   Could we detect an appropriate line ending in ahwrite() after it's been 
   decompressed and buffer partial lines accordingly?
   
   cheers
   
   andrew
   
   

3. Re: pg_restore direct to database is broken for --insert dumps

   Tom Lane <tgl@sss.pgh.pa.us> — 2012-01-04T23:20:19Z

   Andrew Dunstan <andrew@dunslane.net> writes:
   > On 01/04/2012 01:13 PM, Tom Lane wrote:
   >> Not entirely sure what to do about this.  We could consider reverting
   >> the aforesaid patch and trying to find another way of fixing that code's
   >> failure to cope with standard-conforming strings, but I'm not sure that
   >> there's a good way to know whether standard_conforming_strings is active
   >> here, and anyway that code was ugly as sin and I'd rather not resurrect
   >> it.  But on the other hand, there are no clear line boundaries in the
   >> compressed data, and we can't introduce any without (a) worsening
   >> compression and (b) breaking compatibility with existing dump files.
   >> 
   >> Anybody have any bright ideas?  I'm fresh out at the moment.
   
   > I pondered this while out on my daily constitutional. The first thing 
   > that occurred to me is that it would possibly have been better if we'd 
   > made pg_dump not use a quoting mechanism whose behaviour is dependent on 
   > a setting (e.g. E'' or dollar quoting). But I guess that's water under 
   > the bridge.
   
   > Could we detect an appropriate line ending in ahwrite() after it's been 
   > decompressed and buffer partial lines accordingly?
   
   Not easily: there could be newlines embedded in data strings or SQL
   identifiers.  I'm not seeing any way around this except to restore the
   minimal lexing capability.  One thing we could probably do is to
   restrict it to be used only when reading table data, and continue to
   assume that object creation commands can be emitted as-is.  That would
   at least get us out of needing to parse dollar-quoted literals, which
   aren't used in the INSERT commands.  But we'd have to deal with
   standard-conforming strings some way.  The idea I had about that, since
   we have an open database connection at hand, is to check the connected
   backend's standard_conforming_strings state via PQparameterStatus.  If
   it doesn't have the right setting then things are going to fail anyway.
   
   			regards, tom lane
   
   

4. Re: pg_restore direct to database is broken for --insert dumps

   Andrew Dunstan <andrew@dunslane.net> — 2012-01-04T23:59:17Z

   
   On 01/04/2012 06:20 PM, Tom Lane wrote:
   > But we'd have to deal with
   > standard-conforming strings some way.  The idea I had about that, since
   > we have an open database connection at hand, is to check the connected
   > backend's standard_conforming_strings state via PQparameterStatus.  If
   > it doesn't have the right setting then things are going to fail anyway.
   >
   
   Do we care what it is? Or can we just issue a SET to make it what it 
   needs to be?
   
   cheers
   
   andrew
   
   

5. Re: pg_restore direct to database is broken for --insert dumps

   Tom Lane <tgl@sss.pgh.pa.us> — 2012-01-05T00:15:25Z

   Andrew Dunstan <andrew@dunslane.net> writes:
   > On 01/04/2012 06:20 PM, Tom Lane wrote:
   >> But we'd have to deal with
   >> standard-conforming strings some way.  The idea I had about that, since
   >> we have an open database connection at hand, is to check the connected
   >> backend's standard_conforming_strings state via PQparameterStatus.  If
   >> it doesn't have the right setting then things are going to fail anyway.
   
   > Do we care what it is?
   
   Well, yeah, we care.
   
   > Or can we just issue a SET to make it what it needs to be?
   
   We already did, but I don't think the code in pg_backup_db has
   convenient access to the value.  I might be wrong about that.
   
   			regards, tom lane
   
   

6. Re: pg_restore direct to database is broken for --insert dumps

   Andrew Dunstan <andrew@dunslane.net> — 2012-01-05T14:51:07Z

   
   On 01/04/2012 06:20 PM, Tom Lane wrote:
   >> Could we detect an appropriate line ending in ahwrite() after it's been
   >> decompressed and buffer partial lines accordingly?
   > Not easily: there could be newlines embedded in data strings or SQL
   > identifiers.
   > 	
   
   
   Should we look at eliminating those newlines for the future by using 
   U&"" identifiers where there are embedded newlines and unicode escapes 
   for newlines in data strings?
   
   Then at least we'd possibly be able to get rid of the kludge some time 
   in the future.
   
   
   cheers
   
   andrew
   
   

7. Re: pg_restore direct to database is broken for --insert dumps

   Tom Lane <tgl@sss.pgh.pa.us> — 2012-01-05T15:44:28Z

   Andrew Dunstan <andrew@dunslane.net> writes:
   > On 01/04/2012 06:20 PM, Tom Lane wrote:
   >> Not easily: there could be newlines embedded in data strings or SQL
   >> identifiers.
   
   > Should we look at eliminating those newlines for the future by using 
   > U&"" identifiers where there are embedded newlines and unicode escapes 
   > for newlines in data strings?
   
   > Then at least we'd possibly be able to get rid of the kludge some time 
   > in the future.
   
   Given our high expectations for forward-compatibility of dump files, we
   couldn't drop the code anytime in the foreseeable future, so I cannot
   get excited about spending time on that.  (AFAIR, we have *never*
   intentionally broken compatibility of pg_dump files, unless you count
   breaking dumps made by unreleased development versions.)
   
   			regards, tom lane
   
   

8. Re: pg_restore direct to database is broken for --insert dumps

   Tom Lane <tgl@sss.pgh.pa.us> — 2012-01-06T00:01:23Z

   I wrote:
   > Not easily: there could be newlines embedded in data strings or SQL
   > identifiers.  I'm not seeing any way around this except to restore the
   > minimal lexing capability.  One thing we could probably do is to
   > restrict it to be used only when reading table data, and continue to
   > assume that object creation commands can be emitted as-is.  That would
   > at least get us out of needing to parse dollar-quoted literals, which
   > aren't used in the INSERT commands.
   
   Attached is a patch that restores a much-simplified version of the mini
   lexer; it deals only with the sorts of things that dumpTableData_insert
   actually emits.  Fixing the standard_conforming_strings issue turns out
   to be really a one-liner, because the setting is in fact readily
   available to this code.  Maybe we should have done it that way to begin
   with :-( ... though I admit to being glad to have gotten rid of the very
   questionable dollar-quote-recognition code that was there before.
   
   Barring objections to the approach, I'll apply and back-patch this
   tomorrow.
   
   			regards, tom lane