pg_dump is broken in CVS tip

Started by Tom Lanealmost 24 years ago10 messages
#1Tom Lane
tgl@sss.pgh.pa.us

pg_dumping a table having a primary key yields commands like

--
-- TOC Entry ID 2 (OID 139812)
--
-- Name: table1 Type: TABLE Owner: postgres
--

CREATE TABLE "table1" (
"column10" character varying(255) NOT NULL,
"column1" character varying(255) NOT NULL,
"column2" smallint NOT NULL,
"column6" numeric,
"column7" "char",
Constraint "table1_pkey" Primary Key ("column10", "column1", "column2")
);

[snip]

--
-- TOC Entry ID 5 (OID 139817)
--
-- Name: "table1_pkey" Type: CONSTRAINT Owner: postgres
--

Alter Table "table1" Add Constraint "table1_pkey" Primary Key ("column10", "column1", "column2");

which on execution quite properly complains about duplicate primary
keys.

I assume this is traceable to this patch:

2002-03-06 15:48 momjian

* src/bin/pg_dump/pg_dump.c: Enable ALTER TABLE ADD PRIMARY KEY for
pg_dump, for performance reasons so index is not on table during
COPY.

AFAICT, the patch I posted to -patches a little while to enable

the

usage of ALTER TABLE ADD PRIMARY KEY by pg_dump hasn't been

applied, nor

is it in the unapplied patches list. I was under the impression

that

this was in the queue for application -- did it just get lost?

Neil Conway <neilconway@rogers.com>

It would seem that more thought is needed here.

regards, tom lane

#2Neil Conway
neilconway@rogers.com
In reply to: Tom Lane (#1)
Re: pg_dump is broken in CVS tip

On Fri, 12 Apr 2002 13:28:34 -0400
"Tom Lane" <tgl@sss.pgh.pa.us> wrote:

pg_dumping a table having a primary key yields commands like

--
-- TOC Entry ID 2 (OID 139812)
--
-- Name: table1 Type: TABLE Owner: postgres
--

CREATE TABLE "table1" (
"column10" character varying(255) NOT NULL,
"column1" character varying(255) NOT NULL,
"column2" smallint NOT NULL,
"column6" numeric,
"column7" "char",
Constraint "table1_pkey" Primary Key ("column10", "column1", "column2")
);

[snip]

--
-- TOC Entry ID 5 (OID 139817)
--
-- Name: "table1_pkey" Type: CONSTRAINT Owner: postgres
--

Alter Table "table1" Add Constraint "table1_pkey" Primary Key ("column10", "column1", "column2");

which on execution quite properly complains about duplicate primary
keys.

Thanks for finding this Tom -- my apologies, this is likely my bug.

However, when I created a table using the commands above and then
dumped it again, I got a dump that worked properly: there was no
Constraint within the table definition itself, just an ALTER
TABLE at the end of the dump to add the PK (i.e. the patch worked
as intended and the table could be restored properly).

If you can give me a reproduceable test-case, I'll fix the bug.

Cheers,

Neil

--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Neil Conway (#2)
Re: pg_dump is broken in CVS tip

Neil Conway <neilconway@rogers.com> writes:

However, when I created a table using the commands above and then
dumped it again, I got a dump that worked properly:
...
If you can give me a reproduceable test-case, I'll fix the bug.

Sigh ... I should take my own advice about checking that I've described
a problem completely :-(. It looks like you also need a foreign-key
reference to the table. This will generate the problem:

create table t1 (f1 int primary key);

create table t2 (f1 int references t1);

The dump of t1 will now read

CREATE TABLE "t1" (
"f1" integer NOT NULL,
Constraint "t1_pkey" Primary Key ("f1")
);

Sorry for the inadequate report.

regards, tom lane

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#3)
Re: pg_dump is broken in CVS tip

I said:

This will generate the problem:

create table t1 (f1 int primary key);

create table t2 (f1 int references t1);

Actually, I find that I get the double declaration of t1_pkey even
without t2. Either we're not using quite the same sources, or the
problem is platform-dependent. I can dig into it if you cannot
reproduce it ...

regards, tom lane

#5Neil Conway
nconway@klamath.dyndns.org
In reply to: Tom Lane (#4)
Re: pg_dump is broken in CVS tip

On Fri, 12 Apr 2002 18:59:33 -0400
"Tom Lane" <tgl@sss.pgh.pa.us> wrote:

I said:

This will generate the problem:

create table t1 (f1 int primary key);

create table t2 (f1 int references t1);

Actually, I find that I get the double declaration of t1_pkey even
without t2. Either we're not using quite the same sources, or the
problem is platform-dependent. I can dig into it if you cannot
reproduce it ...

Curious -- I was previously using ~1 week old sources, and I was
unable to reproduce the problem (using either the original
test-case or the one provided above: neither has any problems).
When I built the current CVS code, both test-case exhibits the
problem quite obviously. Therefore, it seems that the problem
has been introduced recently.

I'll investigate...

Cheers,

Neil

P.S. Tom, would you mind adding my IP to your spam whitelist?
Your spam-blocking software rejects my emails.

--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

#6Neil Conway
nconway@klamath.dyndns.org
In reply to: Neil Conway (#5)
1 attachment(s)
Re: pg_dump is broken in CVS tip

On Fri, 12 Apr 2002 19:24:21 -0400
"Neil Conway" <nconway@klamath.dyndns.org> wrote:

When I built the current CVS code, both test-case exhibits the
problem quite obviously. Therefore, it seems that the problem
has been introduced recently.

The problem was introduced here:

------------------
revision 1.246
date: 2002/04/05 11:51:12; author: momjian; state: Exp; lines: +129 -3
Adds domain dumping support to pg_dump.

Rod Taylor
------------------

Rod's patch does what it is supposed to do, but it also includes
some old code to add PK constraints to CREATE TABLE. That stuff
had been removed as part of my original patch for pg_dump a
little while ago.

The attached patch fixes this by removing (again :-) ) the
code in dumpTables() to perform PK creation during CREATE
TABLE. I briefly tested it locally and it fixes both of
Tom's test cases.

Please apply.

Cheers,

Neil

--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

Attachments:

pg_dump_fix.patchapplication/octet-stream; name=pg_dump_fix.patchDownload
Index: src/bin/pg_dump/pg_dump.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.247
diff -c -r1.247 pg_dump.c
*** src/bin/pg_dump/pg_dump.c	11 Apr 2002 20:00:06 -0000	1.247
--- src/bin/pg_dump/pg_dump.c	13 Apr 2002 01:21:02 -0000
***************
*** 4418,4453 ****
  									  tblinfo[i].check_expr[k]);
  				}
  
- 				/* Primary Key */
- 				if (tblinfo[i].pkIndexOid != NULL)
- 				{
- 					PQExpBuffer consDef;
- 
- 					/* Find the corresponding index */
- 					for (k = 0; k < numIndexes; k++)
- 					{
- 						if (strcmp(indinfo[k].indexreloid,
- 								   tblinfo[i].pkIndexOid) == 0)
- 							break;
- 					}
- 
- 					if (k >= numIndexes)
- 					{
- 						write_msg(NULL, "dumpTables(): failed sanity check, could not find index (%s) for primary key constraint\n",
- 								  tblinfo[i].pkIndexOid);
- 						exit_nicely();
- 					}
- 
- 					consDef = getPKconstraint(&tblinfo[i], &indinfo[k]);
- 
- 					if ((actual_atts + tblinfo[i].ncheck) > 0)
- 						appendPQExpBuffer(q, ",\n\t");
- 
- 					appendPQExpBuffer(q, "%s", consDef->data);
- 
- 					destroyPQExpBuffer(consDef);
- 				}
- 
  				/*
  				 * Primary Key: In versions of PostgreSQL prior to 7.2, we
  				 * needed to include the primary key in the table definition.
--- 4418,4423 ----
#7Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Neil Conway (#6)
Re: pg_dump is broken in CVS tip

I will apply shortly because pg_dump is broken. I will give it 8 hours.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Neil Conway wrote:

On Fri, 12 Apr 2002 19:24:21 -0400
"Neil Conway" <nconway@klamath.dyndns.org> wrote:

When I built the current CVS code, both test-case exhibits the
problem quite obviously. Therefore, it seems that the problem
has been introduced recently.

The problem was introduced here:

------------------
revision 1.246
date: 2002/04/05 11:51:12; author: momjian; state: Exp; lines: +129 -3
Adds domain dumping support to pg_dump.

Rod Taylor
------------------

Rod's patch does what it is supposed to do, but it also includes
some old code to add PK constraints to CREATE TABLE. That stuff
had been removed as part of my original patch for pg_dump a
little while ago.

The attached patch fixes this by removing (again :-) ) the
code in dumpTables() to perform PK creation during CREATE
TABLE. I briefly tested it locally and it fixes both of
Tom's test cases.

Please apply.

Cheers,

Neil

--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Neil Conway (#5)
Re: pg_dump is broken in CVS tip

Neil Conway <nconway@klamath.dyndns.org> writes:

Curious -- I was previously using ~1 week old sources, and I was
unable to reproduce the problem (using either the original
test-case or the one provided above: neither has any problems).
When I built the current CVS code, both test-case exhibits the
problem quite obviously. Therefore, it seems that the problem
has been introduced recently.

[ scratches head ... ] I think most of the major recent changes
have been schema related, so this is quite likely my fault. Let
me know if you need help debugging it.

P.S. Tom, would you mind adding my IP to your spam whitelist?
Your spam-blocking software rejects my emails.

24.102.202.* whitelisted; let me know if that's not the correct
IP range for you. (Although you might think I'm blocking most
of the net, I still get a depressingly large amount of spam ---
and that's not even counting the Klez virus that seems to be
deliberately targeting my jpeg-info alter ego ... grumble ...)

regards, tom lane

#9Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#8)
Re: pg_dump is broken in CVS tip

Tom Lane wrote:

Neil Conway <nconway@klamath.dyndns.org> writes:

Curious -- I was previously using ~1 week old sources, and I was
unable to reproduce the problem (using either the original
test-case or the one provided above: neither has any problems).
When I built the current CVS code, both test-case exhibits the
problem quite obviously. Therefore, it seems that the problem
has been introduced recently.

[ scratches head ... ] I think most of the major recent changes
have been schema related, so this is quite likely my fault. Let
me know if you need help debugging it.

Tom, did you see his patch posted a few hours ago. Did that fix it? I
can apply.

P.S. Tom, would you mind adding my IP to your spam whitelist?
Your spam-blocking software rejects my emails.

24.102.202.* whitelisted; let me know if that's not the correct
IP range for you. (Although you might think I'm blocking most
of the net, I still get a depressingly large amount of spam ---
and that's not even counting the Klez virus that seems to be
deliberately targeting my jpeg-info alter ego ... grumble ...)

Have you read my spam blocking article and tools:

http://candle.pha.pa.us/main/writings/spam

It blocks ~70-80% with no false blocks.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#10Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Neil Conway (#6)
Re: pg_dump is broken in CVS tip

Patch applied. Thanks.

---------------------------------------------------------------------------

Neil Conway wrote:

On Fri, 12 Apr 2002 19:24:21 -0400
"Neil Conway" <nconway@klamath.dyndns.org> wrote:

When I built the current CVS code, both test-case exhibits the
problem quite obviously. Therefore, it seems that the problem
has been introduced recently.

The problem was introduced here:

------------------
revision 1.246
date: 2002/04/05 11:51:12; author: momjian; state: Exp; lines: +129 -3
Adds domain dumping support to pg_dump.

Rod Taylor
------------------

Rod's patch does what it is supposed to do, but it also includes
some old code to add PK constraints to CREATE TABLE. That stuff
had been removed as part of my original patch for pg_dump a
little while ago.

The attached patch fixes this by removing (again :-) ) the
code in dumpTables() to perform PK creation during CREATE
TABLE. I briefly tested it locally and it fixes both of
Tom's test cases.

Please apply.

Cheers,

Neil

--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026