Hi
2014-08-14 9:03 GMT+02:00 Heikki Linnakangas <hlinnakan...@vmware.com>:
> On 08/14/2014 06:53 AM, Joachim Wieland wrote:
>
>> I'm seeing an assertion failure with "pg_dump -c --if-exists" which is
>> not ready to handle BLOBs it seems:
>>
>> pg_dump: pg_backup_archiver.c:472: RestoreArchive: Assertion `mark !=
>> ((void *)0)' failed.
>>
>> To reproduce:
>>
>> $ createdb test
>> $ pg_dump -c --if-exists test (works, dumps empty database)
>> $ psql test -c "select lo_create(1);"
>> $ pg_dump -c --if-exists test (fails, with the above mentioned assertion)
>>
>
> The code tries to inject an "IF EXISTS" into the already-construct DROP
> command, but it doesn't work for large objects, because the deletion
> command looks like "SELECT pg_catalog.lo_unlink(xxx)". There is no DROP
> there.
>
> I believe we could use "SELECT pg_catalog.lo_unlink(loid) FROM
> pg_catalog.pg_largeobject_metadata WHERE loid = xxx".
> pg_largeobject_metadata table didn't exist before version 9.0, but we don't
> guarantee pg_dump's output to be compatible in that direction anyway, so I
> think that's fine.
>
> The quick fix would be to add an exception for blobs, close to where
> Assert is. There are a few exceptions there already. A cleaner solution
> would be to add a new argument to ArchiveEntry and make the callers
> responsible for providing an "DROP IF EXISTS" query, but that's not too
> appetizing because for most callers it would be boring boilerplate code.
> Perhaps add an argument, but if it's NULL, ArchiveEntry would form the
> if-exists query automatically from the DROP query.
>
I am sending two patches
first is fast fix
second fix is implementation of Heikki' idea.
> - Heikki
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
commit 68ba9d3b682c6e56b08d623ebc58e351ce1a6c55
Author: Pavel Stehule <pavel.steh...@gooddata.com>
Date: Fri Aug 15 13:41:24 2014 +0200
heikki-lo-if-exists-fix
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 3aebac8..0049be7 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -429,60 +429,100 @@ RestoreArchive(Archive *AHX)
}
else
{
- char buffer[40];
- char *mark;
- char *dropStmt = pg_strdup(te->dropStmt);
- char *dropStmtPtr = dropStmt;
- PQExpBuffer ftStmt = createPQExpBuffer();
-
/*
- * Need to inject IF EXISTS clause after ALTER TABLE
- * part in ALTER TABLE .. DROP statement
+ * LO doesn't use DDL stmts, instead it uses lo_unlink
+ * functions. But it should be injected too.
*/
- if (strncmp(dropStmt, "ALTER TABLE", 11) == 0)
+ if (strncmp(te->desc, "BLOB", 4) == 0)
{
- appendPQExpBuffer(ftStmt,
- "ALTER TABLE IF EXISTS");
- dropStmt = dropStmt + 11;
+ Oid loid = InvalidOid;
+ char *mark;
+
+ /* find and take foid */
+ if (strncmp(te->dropStmt, "SELECT pg_catalog.lo_unlink('", 29) == 0)
+ {
+ unsigned long cvt;
+ char *endptr;
+
+ mark = te->dropStmt + 29;
+
+ if (*mark != '\0')
+ {
+ errno = 0;
+ cvt = strtoul(mark, &endptr, 10);
+
+ /* Ensure we have a valid Oid */
+ if (errno == 0 && mark != endptr && *endptr == '\'')
+ loid = (Oid) cvt;
+ }
+ }
+
+ if (loid == InvalidOid)
+ exit_horribly(modulename, "cannot to identify oid of large object\n");
+
+ /*
+ * Now we have valid foid and we can generate
+ * fault tolerant (not existency) SQL statement.
+ */
+ DropBlobIfExists(AH, loid);
}
-
- /*
- * ALTER TABLE..ALTER COLUMN..DROP DEFAULT does not
- * support the IF EXISTS clause, and therefore we
- * simply emit the original command for such objects.
- * For other objects, we need to extract the first
- * part of the DROP which includes the object type.
- * Most of the time this matches te->desc, so search
- * for that; however for the different kinds of
- * CONSTRAINTs, we know to search for hardcoded "DROP
- * CONSTRAINT" instead.
- */
- if (strcmp(te->desc, "DEFAULT") == 0)
- appendPQExpBuffer(ftStmt, "%s", dropStmt);
else
{
- if (strcmp(te->desc, "CONSTRAINT") == 0 ||
- strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
- strcmp(te->desc, "FK CONSTRAINT") == 0)
- strcpy(buffer, "DROP CONSTRAINT");
+ char buffer[40];
+ char *mark;
+ char *dropStmt = pg_strdup(te->dropStmt);
+ char *dropStmtPtr = dropStmt;
+ PQExpBuffer ftStmt = createPQExpBuffer();
+
+ /*
+ * Need to inject IF EXISTS clause after ALTER TABLE
+ * part in ALTER TABLE .. DROP statement
+ */
+ if (strncmp(dropStmt, "ALTER TABLE", 11) == 0)
+ {
+ appendPQExpBuffer(ftStmt,
+ "ALTER TABLE IF EXISTS");
+ dropStmt = dropStmt + 11;
+ }
+
+ /*
+ * ALTER TABLE..ALTER COLUMN..DROP DEFAULT does not
+ * support the IF EXISTS clause, and therefore we
+ * simply emit the original command for such objects.
+ * For other objects, we need to extract the first
+ * part of the DROP which includes the object type.
+ * Most of the time this matches te->desc, so search
+ * for that; however for the different kinds of
+ * CONSTRAINTs, we know to search for hardcoded "DROP
+ * CONSTRAINT" instead.
+ */
+ if (strcmp(te->desc, "DEFAULT") == 0)
+ appendPQExpBuffer(ftStmt, "%s", dropStmt);
else
- snprintf(buffer, sizeof(buffer), "DROP %s",
- te->desc);
+ {
+ if (strcmp(te->desc, "CONSTRAINT") == 0 ||
+ strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
+ strcmp(te->desc, "FK CONSTRAINT") == 0)
+ strcpy(buffer, "DROP CONSTRAINT");
+ else
+ snprintf(buffer, sizeof(buffer), "DROP %s",
+ te->desc);
- mark = strstr(dropStmt, buffer);
- Assert(mark != NULL);
+ mark = strstr(dropStmt, buffer);
+ Assert(mark != NULL);
- *mark = '\0';
- appendPQExpBuffer(ftStmt, "%s%s IF EXISTS%s",
- dropStmt, buffer,
- mark + strlen(buffer));
- }
+ *mark = '\0';
+ appendPQExpBuffer(ftStmt, "%s%s IF EXISTS%s",
+ dropStmt, buffer,
+ mark + strlen(buffer));
+ }
- ahprintf(AH, "%s", ftStmt->data);
+ ahprintf(AH, "%s", ftStmt->data);
- destroyPQExpBuffer(ftStmt);
+ destroyPQExpBuffer(ftStmt);
- pg_free(dropStmtPtr);
+ pg_free(dropStmtPtr);
+ }
}
}
}
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..3c2ee84
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** RestoreArchive(Archive *AHX)
*** 435,440 ****
--- 435,443 ----
char *dropStmtPtr = dropStmt;
PQExpBuffer ftStmt = createPQExpBuffer();
+ if (strncmp(te->desc, "BLOB", 4) == 0)
+ exit_horribly(modulename, "large objects doesn't support --if-exists option\n");
+
/*
* Need to inject IF EXISTS clause after ALTER TABLE
* part in ALTER TABLE .. DROP statement
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
