pg_dump behaves differently for different archive formats
Restoring a "plain format" dump and a "custom format" dump of
the same database can lead to different results:
pg_dump organizes the SQL statements it creates in "TOC entries".
If a custom format dump is restored with pg_restore, all
SQL statements in a TOC entry will be executed as a single command
and thus in a single transaction.
On the other hand, each SQL statement in a plain format dump
is executed individually in its own transaction, and TOC entries
are irrelevant (except as comments for documentation).
E.g., if a table has ACL entries for several roles and one of
them is not present in the destination database, a plain format
dump will restore all privileges except the ones that pertain
to the missing user, while a custom format dump will not restore
any privileges even for existing users.
This is because all ACL related statements are in one TOC entry.
Another example is a table that you try to restore into a database
where the original table owner does not exist.
With a plain format dump, the table is created, but will belong
to the user restoring the dump, while a custom format dump will
not create the table at all.
This is because CREATE TABLE and ALTER TABLE ... OWNER TO
are in the same TOC entry.
One can argue for or against each individual behaviour, but I
am surprised by the difference.
Is there a deeper reason why it should remain like this or should
I consider it a bug that should get fixed?
Yours,
Laurenz Albe
--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general
Albe Laurenz <laurenz.albe@wien.gv.at> writes:
Restoring a "plain format" dump and a "custom format" dump of
the same database can lead to different results:
pg_dump organizes the SQL statements it creates in "TOC entries".
If a custom format dump is restored with pg_restore, all
SQL statements in a TOC entry will be executed as a single command
and thus in a single transaction.
Yeah, this is a bug I think. pg_dump was designed around the idea
that the output would be executed as a simple script, and in a
number of places there's an expectation that one SQL statement
can fail without affecting following ones. So if pg_restore can't
provide that behavior it's not good.
On the other hand, I'm not sure how much enthusiasm there'd be for
complex or fragile changes to fix this. A lot of people invariably
run restores in single-transaction mode and don't really care about
fault-tolerant restores. Also, it's easy enough to dodge the problem
if you must: just pipe the output into psql rather than
direct-to-database.
So to me the question is can we fix this without doing something like
duplicating psql's lexer? If we have to parse out the statements
contained in each text blob, it's probably going to be too painful.
Some cautionary history about this sort of thing can be read at
/messages/by-id/18006.1325700782@sss.pgh.pa.us
regards, tom lane
--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general
Tom Lane wrote on Dec 16, 2013:
Albe Laurenz <laurenz.albe@wien.gv.at> writes:
Restoring a "plain format" dump and a "custom format" dump of
the same database can lead to different results:
pg_dump organizes the SQL statements it creates in "TOC entries".
If a custom format dump is restored with pg_restore, all
SQL statements in a TOC entry will be executed as a single command
and thus in a single transaction.
Yeah, this is a bug I think. pg_dump was designed around the idea
that the output would be executed as a simple script, and in a
number of places there's an expectation that one SQL statement
can fail without affecting following ones. So if pg_restore can't
provide that behavior it's not good.On the other hand, I'm not sure how much enthusiasm there'd be for
complex or fragile changes to fix this. A lot of people invariably
run restores in single-transaction mode and don't really care about
fault-tolerant restores. Also, it's easy enough to dodge the problem
if you must: just pipe the output into psql rather than
direct-to-database.So to me the question is can we fix this without doing something like
duplicating psql's lexer? If we have to parse out the statements
contained in each text blob, it's probably going to be too painful.
Some cautionary history about this sort of thing can be read at
/messages/by-id/18006.1325700782@sss.pgh.pa.us
I thought that changing the dump format for this would be too
much trouble, so I came up with the attached.
It assumes that custom- or tar-format archives are written by pg_dump
and cannot contain arbitrary SQL statements, which allows me to get away
with very simple parsing.
If this is not shot down immediately on account of fragility, I'd
add it to the next commitfest page.
The problem has been a pain point for my co-workers in the past;
using single-transaction mode doesn't work for us, since we have custom objects
in our template database that cause expected errors when a dump is restored.
Yours,
Laurenz Albe
Attachments:
pg_restore.patchapplication/octet-stream; name=pg_restore.patchDownload
Make pg_restore apply TOC entries statement by statement.
Previously, the whole TOC entry was restored with a single PQexec, which caused all statements to fail if one of them failed.
This behaviour was different from restoring a plain format dump, where the statements were restored individually.
---
src/bin/pg_dump/pg_backup_archiver.c | 131 +++++++++++++++++++++++++++++++++-
1 files changed, 130 insertions(+), 1 deletions(-)
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
new file mode 100644
index 3aebac8..dd5530f
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** static ArchiveHandle *_allocAH(const cha
*** 55,60 ****
--- 55,61 ----
const int compression, ArchiveMode mode, SetupWorkerPtr setupWorkerPtr);
static void _getObjectDescription(PQExpBuffer buf, TocEntry *te,
ArchiveHandle *AH);
+ static char *_nextStatement(char **p);
static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass);
static char *replace_line_endings(const char *str);
static void _doSetFixedOutputState(ArchiveHandle *AH);
*************** _printTocEntry(ArchiveHandle *AH, TocEnt
*** 3177,3183 ****
else
{
if (strlen(te->defn) > 0)
! ahprintf(AH, "%s\n\n", te->defn);
}
/*
--- 3178,3205 ----
else
{
if (strlen(te->defn) > 0)
! {
! /*
! * Break up the definition into individual statements and issue
! * them one by one. If we didn't do that, plain format dumps
! * would behave differently, because there the dump is restored
! * statement by statement. This can cause different behaviour
! * if one of the statements fails.
! */
! char *p = te->defn;
!
! while (*p != '\0')
! {
! char *stmt = _nextStatement(&p);
!
! /* append newlines only to the last statement */
! if (*p == '\0')
! ahprintf(AH, "%s\n\n", stmt);
! else
! ahprintf(AH, "%s", stmt);
! free(stmt);
! }
! }
}
/*
*************** _printTocEntry(ArchiveHandle *AH, TocEnt
*** 3252,3257 ****
--- 3274,3386 ----
}
/*
+ * Parse *command and extract the next SQL statement.
+ * The next statement is returned in a newly allocated string,
+ * and *command is changed to point to the beginning of the next statement.
+ */
+ static char *
+ _nextStatement(char **command)
+ {
+ char *p, *q, *ret, h, *message;
+ int status = 0; /* 0 normal, 1 single quotes, 2 double quotes, 3 problem */
+
+ for (p = *command; *p != '\0' && status != 3 && (*p != ';' || status != 0); ++p)
+ {
+ switch (*p)
+ {
+ case '\'':
+ if (status == 0)
+ status = 1;
+ else if (status == 1)
+ status = 0;
+ break;
+ case '"':
+ if (status == 0)
+ status = 2;
+ else if (status == 2)
+ status = 0;
+ break;
+ case '$':
+ if (status != 0)
+ continue;
+
+ /*
+ * We don't need a full-fledged parser for dollar quotes
+ * here because we can assume that the dump file was written
+ * by pg_dump.
+ * We assume that all names containing a dollar sign will be double
+ * quoted and that no parameters like $1 will be in SQL statements.
+ * Then every occurrence of a dollar sign outside quotes would be
+ * part of a dollar quote.
+ */
+ for (q = p + 1; *q != '\0' && *q != '$'; ++q)
+ ;
+ if (*q == '$')
+ {
+ char *limit;
+
+ /* copy delimiter including dollars */
+ h = *(q + 1);
+ *(q + 1) = '\0';
+ limit = pg_strdup(p);
+ *(q + 1) = h;
+
+ /* find next occurrence of delimiter */
+ q = strstr(q + 1, limit);
+
+ /* skip quoted string if found */
+ if (q)
+ p = q + (strlen(limit) - 1);
+ else
+ {
+ message = "unfinished dollar quote";
+ status = 3;
+ }
+
+ free(limit);
+ }
+ else
+ {
+ message = "dollar sign is not part of a dollar quote";
+ status = 3;
+ }
+ break;
+ default:
+ continue;
+ }
+ }
+
+ /* skip to the beginning of the next non-whitespace */
+ while (*p == ';' || *p == '\n' || *p == '\r' || *p == ' ')
+ ++p;
+
+ if (*p == '\0' && status != 0)
+ {
+ message = "unfinished quote";
+ status = 3;
+ }
+
+ if (status == 3)
+ {
+ write_msg(modulename, "WARNING: %s", message);
+
+ /* don't split the command string if there was a parsing problem */
+ ret = pg_strdup(*command);
+ *command += strlen(*command);
+ }
+ else
+ {
+ h = *p;
+ *p = '\0';
+ ret = pg_strdup(*command);
+ *p = h;
+ *command = p;
+ }
+
+ return ret;
+ }
+
+ /*
* Sanitize a string to be included in an SQL comment, by replacing any
* newlines with spaces.
*/
--
1.7.1
Albe Laurenz <laurenz.albe@wien.gv.at> writes:
I thought that changing the dump format for this would be too
much trouble, so I came up with the attached.
It assumes that custom- or tar-format archives are written by pg_dump
and cannot contain arbitrary SQL statements, which allows me to get away
with very simple parsing.
I don't think this can be trusted in the least. To begin with, where'd
you get the idea dumps cannot contain "arbitrary SQL statements"? CREATE
RULE at least could contain some pretty weird stuff. This thing doesn't
look like it's even bothering to count nested parentheses, so it will
certainly fail on a multi-statement rule. I believe you're also at risk
of SQL injection attacks from failing to account for multibyte characters
in non-ASCII-safe client encodings.
While those specific problems could no doubt be fixed, I object to the
entire concept of assuming that what pg_dump emits is always going to be
trivially parsable. If we are to go down this path, I think we have to
replicate what psql is doing to identify statement boundaries ... and
as I mentioned upthread, that's rather a lot of code :-(
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
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Albe Laurenz <laurenz.albe@wien.gv.at> writes:
I thought that changing the dump format for this would be too
much trouble, so I came up with the attached.
If we're going to change this, it seems to me that the only option would
be to change the dump format... Just off-the-cuff, I'm wondering if we
could actually not change the real 'format' but simply promote each ACL
entry (and similar cases..) to top-level objects and declare that TOC
entries should be single statements.
While those specific problems could no doubt be fixed, I object to the
entire concept of assuming that what pg_dump emits is always going to be
trivially parsable. If we are to go down this path, I think we have to
replicate what psql is doing to identify statement boundaries ... and
as I mentioned upthread, that's rather a lot of code :-(
Agreed. If we want this, we should handle it on the pg_dump side, not
try and work it out on the pg_restore side.
Thanks,
Stephen
Stephen Frost <sfrost@snowman.net> writes:
If we're going to change this, it seems to me that the only option would
be to change the dump format... Just off-the-cuff, I'm wondering if we
could actually not change the real 'format' but simply promote each ACL
entry (and similar cases..) to top-level objects and declare that TOC
entries should be single statements.
I don't think we want even more TOC entries, but it would not be
unreasonable to insist that the statement(s) within a TOC entry be
subdivided somehow. Essentially the payload of a TOC entry becomes
a list of strings rather than just one string.
That would mean that the problem could not be fixed for existing archive
files; but that seems OK, given the rather small number of complaints
so far.
If we had something like that, I'd be strongly inclined to get rid of
the existing convention whereby comments and ACL commands are separate
TOC entries, and make them part of the parent object's TOC entry (which'd
mean we'd want to label the sub-strings so we can tell whether they are
main object, comment, or ACL). The fewer TOC entries we can have, the
better; there is no reason why comments/ACLs should be independently
sortable.
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
On Mon, Jul 28, 2014 at 10:55 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Stephen Frost <sfrost@snowman.net> writes:
If we're going to change this, it seems to me that the only option would
be to change the dump format... Just off-the-cuff, I'm wondering if we
could actually not change the real 'format' but simply promote each ACL
entry (and similar cases..) to top-level objects and declare that TOC
entries should be single statements.I don't think we want even more TOC entries, but it would not be
unreasonable to insist that the statement(s) within a TOC entry be
subdivided somehow. Essentially the payload of a TOC entry becomes
a list of strings rather than just one string.That would mean that the problem could not be fixed for existing archive
files; but that seems OK, given the rather small number of complaints
so far.If we had something like that, I'd be strongly inclined to get rid of
the existing convention whereby comments and ACL commands are separate
TOC entries, and make them part of the parent object's TOC entry (which'd
mean we'd want to label the sub-strings so we can tell whether they are
main object, comment, or ACL). The fewer TOC entries we can have, the
better; there is no reason why comments/ACLs should be independently
sortable.
Maybe, but I think people will still want an option to skip restoring
them altogether (at least for ACLs).
--
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
Robert Haas <robertmhaas@gmail.com> writes:
On Mon, Jul 28, 2014 at 10:55 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
If we had something like that, I'd be strongly inclined to get rid of
the existing convention whereby comments and ACL commands are separate
TOC entries, and make them part of the parent object's TOC entry (which'd
mean we'd want to label the sub-strings so we can tell whether they are
main object, comment, or ACL). The fewer TOC entries we can have, the
better; there is no reason why comments/ACLs should be independently
sortable.
Maybe, but I think people will still want an option to skip restoring
them altogether (at least for ACLs).
Sure; we already have such an option, and I'm not suggesting removing
it. The implementation would change though: it would have to look at
the individual command labels to see which part(s) of a TOC entry to
print out.
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