pg_restore --clean vs. large object
Hi,
Since pg_restore --clean doesn't delete existing large objects,
restoring to an existing database with "pg_restore --clean -1"
would fail if backup archive contains large objects. Some DBAs
complain to the behavior because they expect all existing data
conflicting with backup archive will be deleted automatically.
I'd like to improve the behavior if it is not intentional.
The attached is a patch to execute lo_unlink() before lo_create()
in pg_restore. To avoid transaction rollbacks, I added a test
whether the large object exists with an EXISTS query.
SELECT CASE WHEN EXISTS
(SELECT 1 FROM pg_catalog.pg_largeobject WHERE loid = %u)
THEN lo_unlink(%u) END;
Comments welcome.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Attachments:
pg_restore_clean_lo.patchapplication/octet-stream; name=pg_restore_clean_lo.patchDownload
diff -cpr head/src/bin/pg_dump/pg_backup_archiver.c pg_restore_clean_lo/src/bin/pg_dump/pg_backup_archiver.c
*** head/src/bin/pg_dump/pg_backup_archiver.c Mon Apr 13 10:34:58 2009
--- pg_restore_clean_lo/src/bin/pg_dump/pg_backup_archiver.c Mon May 18 14:50:04 2009
*************** EndRestoreBlobs(ArchiveHandle *AH)
*** 896,902 ****
* Called by a format handler to initiate restoration of a blob
*/
void
! StartRestoreBlob(ArchiveHandle *AH, Oid oid)
{
Oid loOid;
--- 896,902 ----
* Called by a format handler to initiate restoration of a blob
*/
void
! StartRestoreBlob(ArchiveHandle *AH, Oid oid, bool drop)
{
Oid loOid;
*************** StartRestoreBlob(ArchiveHandle *AH, Oid
*** 906,911 ****
--- 906,914 ----
AH->lo_buf_used = 0;
ahlog(AH, 2, "restoring large object with OID %u\n", oid);
+
+ if (drop)
+ ahprintf(AH, "SELECT CASE WHEN EXISTS(SELECT 1 FROM pg_catalog.pg_largeobject WHERE loid = %u) THEN lo_unlink(%u) END;\n", oid, oid);
if (AH->connection)
{
diff -cpr head/src/bin/pg_dump/pg_backup_archiver.h pg_restore_clean_lo/src/bin/pg_dump/pg_backup_archiver.h
*** head/src/bin/pg_dump/pg_backup_archiver.h Mon Mar 2 09:54:47 2009
--- pg_restore_clean_lo/src/bin/pg_dump/pg_backup_archiver.h Mon May 18 14:50:04 2009
*************** int ReadOffset(ArchiveHandle *, pgoff_
*** 354,360 ****
size_t WriteOffset(ArchiveHandle *, pgoff_t, int);
extern void StartRestoreBlobs(ArchiveHandle *AH);
! extern void StartRestoreBlob(ArchiveHandle *AH, Oid oid);
extern void EndRestoreBlob(ArchiveHandle *AH, Oid oid);
extern void EndRestoreBlobs(ArchiveHandle *AH);
--- 354,360 ----
size_t WriteOffset(ArchiveHandle *, pgoff_t, int);
extern void StartRestoreBlobs(ArchiveHandle *AH);
! extern void StartRestoreBlob(ArchiveHandle *AH, Oid oid, bool drop);
extern void EndRestoreBlob(ArchiveHandle *AH, Oid oid);
extern void EndRestoreBlobs(ArchiveHandle *AH);
diff -cpr head/src/bin/pg_dump/pg_backup_custom.c pg_restore_clean_lo/src/bin/pg_dump/pg_backup_custom.c
*** head/src/bin/pg_dump/pg_backup_custom.c Wed Feb 4 10:05:46 2009
--- pg_restore_clean_lo/src/bin/pg_dump/pg_backup_custom.c Mon May 18 14:50:04 2009
*************** static void _StartBlobs(ArchiveHandle *A
*** 54,60 ****
static void _StartBlob(ArchiveHandle *AH, TocEntry *te, Oid oid);
static void _EndBlob(ArchiveHandle *AH, TocEntry *te, Oid oid);
static void _EndBlobs(ArchiveHandle *AH, TocEntry *te);
! static void _LoadBlobs(ArchiveHandle *AH);
static void _Clone(ArchiveHandle *AH);
static void _DeClone(ArchiveHandle *AH);
--- 54,60 ----
static void _StartBlob(ArchiveHandle *AH, TocEntry *te, Oid oid);
static void _EndBlob(ArchiveHandle *AH, TocEntry *te, Oid oid);
static void _EndBlobs(ArchiveHandle *AH, TocEntry *te);
! static void _LoadBlobs(ArchiveHandle *AH, bool drop);
static void _Clone(ArchiveHandle *AH);
static void _DeClone(ArchiveHandle *AH);
*************** _PrintTocData(ArchiveHandle *AH, TocEntr
*** 501,507 ****
break;
case BLK_BLOBS:
! _LoadBlobs(AH);
break;
default: /* Always have a default */
--- 501,507 ----
break;
case BLK_BLOBS:
! _LoadBlobs(AH, ropt->dropSchema);
break;
default: /* Always have a default */
*************** _PrintData(ArchiveHandle *AH)
*** 622,628 ****
}
static void
! _LoadBlobs(ArchiveHandle *AH)
{
Oid oid;
--- 622,628 ----
}
static void
! _LoadBlobs(ArchiveHandle *AH, bool drop)
{
Oid oid;
*************** _LoadBlobs(ArchiveHandle *AH)
*** 631,637 ****
oid = ReadInt(AH);
while (oid != 0)
{
! StartRestoreBlob(AH, oid);
_PrintData(AH);
EndRestoreBlob(AH, oid);
oid = ReadInt(AH);
--- 631,637 ----
oid = ReadInt(AH);
while (oid != 0)
{
! StartRestoreBlob(AH, oid, drop);
_PrintData(AH);
EndRestoreBlob(AH, oid);
oid = ReadInt(AH);
diff -cpr head/src/bin/pg_dump/pg_backup_files.c pg_restore_clean_lo/src/bin/pg_dump/pg_backup_files.c
*** head/src/bin/pg_dump/pg_backup_files.c Wed Feb 4 10:05:46 2009
--- pg_restore_clean_lo/src/bin/pg_dump/pg_backup_files.c Mon May 18 14:50:04 2009
*************** _LoadBlobs(ArchiveHandle *AH, RestoreOpt
*** 382,388 ****
while (oid != 0)
{
! StartRestoreBlob(AH, oid);
_PrintFileData(AH, fname, ropt);
EndRestoreBlob(AH, oid);
_getBlobTocEntry(AH, &oid, fname);
--- 382,388 ----
while (oid != 0)
{
! StartRestoreBlob(AH, oid, ropt->dropSchema);
_PrintFileData(AH, fname, ropt);
EndRestoreBlob(AH, oid);
_getBlobTocEntry(AH, &oid, fname);
diff -cpr head/src/bin/pg_dump/pg_backup_tar.c pg_restore_clean_lo/src/bin/pg_dump/pg_backup_tar.c
*** head/src/bin/pg_dump/pg_backup_tar.c Mon Mar 30 09:46:35 2009
--- pg_restore_clean_lo/src/bin/pg_dump/pg_backup_tar.c Mon May 18 14:50:04 2009
*************** _LoadBlobs(ArchiveHandle *AH, RestoreOpt
*** 736,742 ****
{
ahlog(AH, 1, "restoring large object OID %u\n", oid);
! StartRestoreBlob(AH, oid);
while ((cnt = tarRead(buf, 4095, th)) > 0)
{
--- 736,742 ----
{
ahlog(AH, 1, "restoring large object OID %u\n", oid);
! StartRestoreBlob(AH, oid, ropt->dropSchema);
while ((cnt = tarRead(buf, 4095, th)) > 0)
{
On Mon, May 18, 2009 at 3:10 AM, Itagaki
Takahiro<itagaki.takahiro@oss.ntt.co.jp> wrote:
The attached is a patch to execute lo_unlink() before lo_create()
in pg_restore.
the patch applies almost cleanly (there are only minor and superfluos
hunks), compiles...
it works as expected...
this patch makes me wonder why we dump or restore an object in
pg_largeobject that has been deleted from the user table that had the
oid... but that is another thing...
i think this one could be applied, just as is... there is no need for
docs, because the issue being fixed is not documented... maybe that
should be in doc of older releases?
--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157
Jaime Casanova <jcasanov@systemguards.com.ec> wrote:
i think this one could be applied, just as is... there is no need for
docs, because the issue being fixed is not documented... maybe that
should be in doc of older releases?
Sure, it was an undocumented behavior. Should we need to add details
of this patch to documentation?
[pg_restore.sgml]
-c
--clean
Clean (drop) database objects before recreating them.
(8.5) Also drop large objects with same oids.
(older) Large objects with same oids are not dropped.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Itagaki Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
Jaime Casanova <jcasanov@systemguards.com.ec> wrote:
i think this one could be applied, just as is... there is no need for
docs, because the issue being fixed is not documented... maybe that
should be in doc of older releases?
Sure, it was an undocumented behavior. Should we need to add details
of this patch to documentation?
The release note entry will be sufficient I think.
regards, tom lane
Itagaki Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
The attached is a patch to execute lo_unlink() before lo_create()
in pg_restore.
Applied with corrections --- you had failed to ensure that pg_dump and
pg_restore produce the same output. I also took the opportunity to
schema-qualify the calls of lo_xxx functions, just to be on the safe
side. (The code already sets search_path, but why not be sure ...)
regards, tom lane