Hi all, thanks for the feedback. I updated the patch now.
Alvaro Herrera [2006-02-25 13:47 -0300]:
> > I improved the patch now to only ignore TABLE DATA for existing tables
> > if '-X ignore-existing-tables' is specified. I also updated the
> > documentation.
>
> Is this really an appropiate description for the behavior? What happens
> if the table is not created for some other reason? Consider for example
> a table using a datatype that couldn't be created.
Right. However, if the table is not present at all, then it makes even
less sense to attempt to restore its data. Therefore I consider this
mainly a documentation issue. I changed the option to
-X no-data-for-failed-tables and described it as
By default, table data objects are restored even if the associated
table could not be successfully created (e. g. because it already
exists). [...]
Tom Lane [2006-02-25 12:18 -0500]:
> Martin Pitt <[EMAIL PROTECTED]> writes:
> > Martin Pitt [2006-02-19 14:39 +0100]:
> >> Since this changes the behaviour of pg_restore, this should probably
> >> become an option, e. g. -D / --ignore-existing-table-data. I'll do
> >> this if you agree to the principle of the current patch.
>
> > I improved the patch now to only ignore TABLE DATA for existing tables
> > if '-X ignore-existing-tables' is specified. I also updated the
> > documentation.
>
> This patch is unbelievably ugly and probably vulnerable to coredumps.
> Please use a cleaner way of disabling the subsequent load than tromping
> all over the TOC datastructure, ie, not this:
>
> > + strcpy (tes->desc,
> > "IGNOREDATA");
It should not segfault, but I agree that this is a bit hackish. The
updated patch completely removes the TABLE DATA node from the linked
list. It does not free its memory, though; I did not find a
free_tocentry() or similar function. However, pg_restore is no daemon,
and without the new option the memory would be allocated, too, so it
does not make much difference. Can anyone give me a hint how to
properly free the struct?
> BTW, I'm pretty sure it fails for tables with same names in different
> schemas, too.
Right, sorry for that. I fixed that, too.
Bruce Momjian [2006-02-28 19:54 -0500]:
> I will clean it up before applying.
Thank you. I hope the updated patch makes that a little bit easier.
> Your patch has been added to the PostgreSQL unapplied patches list at:
>
> http://momjian.postgresql.org/cgi-bin/pgpatches
>
> It will be applied as soon as one of the PostgreSQL committers reviews
> and approves it.
Great, thanks!
Martin
P.S. I also updated the test script to create two namespaces with
identidal table names.
http://people.debian.org/~mpitt/test-pg_restore-existing.sh
--
Martin Pitt http://www.piware.de
Ubuntu Developer http://www.ubuntu.com
Debian Developer http://www.debian.org
In a world without walls and fences, who needs Windows and Gates?
diff -ruN postgresql-8.1.3-old/doc/src/sgml/ref/pg_restore.sgml
postgresql-8.1.3/doc/src/sgml/ref/pg_restore.sgml
--- postgresql-8.1.3-old/doc/src/sgml/ref/pg_restore.sgml 2005-11-01
22:09:50.000000000 +0100
+++ postgresql-8.1.3/doc/src/sgml/ref/pg_restore.sgml 2006-03-03
19:13:50.000000000 +0100
@@ -395,6 +395,20 @@
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-X no-data-for-failed-tables</></term>
+ <listitem>
+ <para>
+ By default, table data objects are restored even if the
+ associated table could not be successfully created (e. g.
+ because it already exists). With this option, such table
+ data is silently ignored. This is useful for dumping and
+ restoring databases with tables which contain auxiliary data
+ for PostgreSQL extensions (e. g. PostGIS).
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
diff -ruN postgresql-8.1.3-old/src/bin/pg_dump/pg_backup_archiver.c
postgresql-8.1.3/src/bin/pg_dump/pg_backup_archiver.c
--- postgresql-8.1.3-old/src/bin/pg_dump/pg_backup_archiver.c 2006-02-05
21:58:57.000000000 +0100
+++ postgresql-8.1.3/src/bin/pg_dump/pg_backup_archiver.c 2006-03-03
19:14:03.000000000 +0100
@@ -268,6 +268,23 @@
_printTocEntry(AH, te, ropt, false, false);
defnDumped = true;
+ /* If we could not create a table, ignore the
respective TABLE DATA if
+ * -X no-data-for-failed-tables is given */
+ if (ropt->noDataForFailedTables && AH->lastErrorTE ==
te && strcmp (te->desc, "TABLE") == 0) {
+ TocEntry *tes, *last;
+
+ ahlog (AH, 1, "table %s could not be created,
will not restore its data\n", te->tag);
+
+ for (last = te, tes = te->next; tes != AH->toc;
last = tes, tes = tes->next) {
+ if (strcmp (tes->desc, "TABLE DATA") ==
0 && strcmp (tes->tag, te->tag) == 0 &&
+ strcmp (tes->namespace ?
tes->namespace : "", te->namespace ? te->namespace : "") == 0) {
+ /* remove this node */
+ last->next = tes->next;
+ break;
+ }
+ }
+ }
+
/* If we created a DB, connect to it... */
if (strcmp(te->desc, "DATABASE") == 0)
{
diff -ruN postgresql-8.1.3-old/src/bin/pg_dump/pg_backup.h
postgresql-8.1.3/src/bin/pg_dump/pg_backup.h
--- postgresql-8.1.3-old/src/bin/pg_dump/pg_backup.h 2005-10-15
04:49:38.000000000 +0200
+++ postgresql-8.1.3/src/bin/pg_dump/pg_backup.h 2006-03-03
19:13:50.000000000 +0100
@@ -106,6 +106,7 @@
char *pghost;
char *username;
int ignoreVersion;
+ int noDataForFailedTables;
int requirePassword;
int exit_on_error;
diff -ruN postgresql-8.1.3-old/src/bin/pg_dump/pg_restore.c
postgresql-8.1.3/src/bin/pg_dump/pg_restore.c
--- postgresql-8.1.3-old/src/bin/pg_dump/pg_restore.c 2006-03-03
19:13:48.000000000 +0100
+++ postgresql-8.1.3/src/bin/pg_dump/pg_restore.c 2006-03-03
19:13:50.000000000 +0100
@@ -254,6 +254,8 @@
use_setsessauth = 1;
else if (strcmp(optarg, "disable-triggers") ==
0)
disable_triggers = 1;
+ else if (strcmp(optarg,
"no-data-for-failed-tables") == 0)
+ opts->noDataForFailedTables = 1;
else
{
fprintf(stderr,
@@ -394,6 +396,8 @@
printf(_(" -X use-set-session-authorization,
--use-set-session-authorization\n"
" use SESSION AUTHORIZATION
commands instead of\n"
" OWNER TO commands\n"));
+ printf(_(" -X no-data-for-failed-tables\n"
+ " do not restore data of tables
which could not be created\n"));
printf(_("\nConnection options:\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket
directory\n"));
signature.asc
Description: Digital signature
