Re: [HACKERS] [GENERAL] Large object corruption during 'piped' pg_restore
Robert Haas writes: > On Fri, Jan 21, 2011 at 12:44 PM, Bosco Rama wrote: >> Tom Lane wrote: >>> So I'm not sure whether to fix it, or leave it as a known failure case >>> in old branches. Comments? >> As an end user there is one area of the DB that I want to work correctly >> 100% of the time and that is the dump/restore tool(s). > Yeah, I lean toward saying we should back-patch this. Fair enough, I'll go do it. I just wanted to hear at least one other person opine that it was worth taking some risk for. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Large object corruption during 'piped' pg_restore
On Fri, Jan 21, 2011 at 12:44 PM, Bosco Rama wrote: > Tom Lane wrote: >> >> So I'm not sure whether to fix it, or leave it as a known failure case >> in old branches. Comments? > > I understand the reluctance to fool with stable code. I have zero insight > into your installed versions distribution and backward compatibility needs > so any comment I may have here is purely selfish. > > As an end user there is one area of the DB that I want to work correctly > 100% of the time and that is the dump/restore tool(s). If it's not going > to work under certain circumstances it should at least tell me so and > fail. I don't think having the tool appear to work while corrupting the > data (even if documented as doing so) is a viable method of operation. Yeah, I lean toward saying we should back-patch this. Yeah, it'll be lightly tested, but it's a pretty confined change, so it's unlikely to break anything else. ISTM the worst case scenario is that it takes two minor releases to get it right, and even that seems fairly unlikely. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Large object corruption during 'piped' pg_restore
Tom Lane wrote: > > So I'm not sure whether to fix it, or leave it as a known failure case > in old branches. Comments? I understand the reluctance to fool with stable code. I have zero insight into your installed versions distribution and backward compatibility needs so any comment I may have here is purely selfish. As an end user there is one area of the DB that I want to work correctly 100% of the time and that is the dump/restore tool(s). If it's not going to work under certain circumstances it should at least tell me so and fail. I don't think having the tool appear to work while corrupting the data (even if documented as doing so) is a viable method of operation. Just my $0.02 Bosco. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Large object corruption during 'piped' pg_restore
On Thu, Jan 20, 2011 at 6:14 PM, Tom Lane wrote: > So I'm not sure whether to fix it, or leave it as a known failure case > in old branches. Comments? Since there is a workaround, I think it is best to document it and leave it as-is. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Large object corruption during 'piped' pg_restore
Bosco Rama writes: >>> If 'standard_conforming_strings = on' is set in our DB (which is required >>> for >>> our app) then the piped restore method (e.g. pg_restore -O backup.dat | >>> psql) >>> results in the large objects being corrupted. > All servers and client tools involved are PG 8.4.6 on Ubuntu Server 10.04.1 > LTS > with all current updates applied. I've been able to replicate this in 8.4; it doesn't happen in 9.0 (but probably does in all 8.x versions). The problem is that pg_dump (or in this case really pg_restore) is relying on libpq's PQescapeBytea() to format the bytea literal that will be given as argument to lowrite() during the restore. When pg_dump is producing SQL directly, or when pg_restore is connected to a database, PQescapeBytea() mooches the standard_conforming_strings value from the active libpq connection and gets the right answer. In the single case where pg_restore is producing SQL without ever opening a database connection, PQescapeBytea doesn't know what to do and defaults to the old non-standard-strings behavior. Unfortunately pg_restore set standard_conforming_strings=on earlier in the script (if it was set in the original source database) so you get the wrong thing. The bottom line is that pg_dump can't depend on libpq's PQescapeBytea, but needs its own copy. We have in fact done that as of 9.0, which is what I was vaguely remembering: Author: Tom Lane Branch: master Release: REL9_0_BR [b1732111f] 2009-08-04 21:56:09 + Fix pg_dump to do the right thing when escaping the contents of large objects. The previous implementation got it right in most cases but failed in one: if you pg_dump into an archive with standard_conforming_strings enabled, then pg_restore to a script file (not directly to a database), the script will set standard_conforming_strings = on but then emit large object data as nonstandardly-escaped strings. At the moment the code is made to emit hex-format bytea strings when dumping to a script file. We might want to change to old-style escaping for backwards compatibility, but that would be slower and bulkier. If we do, it's just a matter of reimplementing appendByteaLiteral(). This has been broken for a long time, but given the lack of field complaints I'm not going to worry about back-patching. I'm not sure whether this new complaint is enough reason to reconsider back-patching. We cannot just backport the 9.0 patch, since it assumes it can do bytea hex output --- we'd need to emit old style escaped output instead. So it's a bit of work, and more to the point would involve pushing poorly-tested code into stable branches. I doubt it would go wrong, but in the worst-case scenario we might create failures for blob-restore cases that work now. So I'm not sure whether to fix it, or leave it as a known failure case in old branches. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers