I've attached a patch which enhances pg_dump to use ALTER TABLE when
defining the primary keys of a table (instead of including this
information in the table definition).

This follows a suggestion by Christopher Kings-Lynne (good idea!).

The patch is pretty simple, partly because the code was already there,
just commented out ;-) I fixed a typo and added some comments.

Bruce suggested that this should get some code review, since a bug in
pg_dump would be bad news. Any comments would be welcome.

Cheers,

Neil

-- 
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC
18 Feb 2002 21:26:05 -0500
Neil Conway <nconway@klamath.dyndns.org>
pg_dump: use ALTER TABLE for PKs
I tried going patch <pg_dump_pk.patch in the root dir of latest pgsql cvs
and got this:

Hmm...  Looks like a context diff to me...
The text leading up to this was:
--------------------------
|*** ./src/bin/pg_dump/pg_dump.c.orig   Mon Feb 18 20:17:59 2002
|--- ./src/bin/pg_dump/pg_dump.c        Mon Feb 18 21:16:37 2002
--------------------------
Patching file ./src/bin/pg_dump/pg_dump.c using Plan A...
Hunk #1 failed at 4251.
Hunk #2 failed at 4261.
(Fascinating--this is really a new-style context diff but without
the telltale extra asterisks on the *** line that usually indicate
the new style...)
Hunk #3 failed at 4454.
3 out of 3 hunks failed--saving rejects to ./src/bin/pg_dump/pg_dump.c.rej
done

How should I apply this patch?

Chris

> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org
> [mailto:pgsql-hackers-owner@postgresql.org]On Behalf Of Neil Conway
> Sent: Tuesday, 19 February 2002 10:26 AM
> To: pgsql-hackers@postgresql.org
> Subject: [HACKERS] pg_dump: use ALTER TABLE for PKs
>
>
> I've attached a patch which enhances pg_dump to use ALTER TABLE when
> defining the primary keys of a table (instead of including this
> information in the table definition).
>
> This follows a suggestion by Christopher Kings-Lynne (good idea!).
>
> The patch is pretty simple, partly because the code was already there,
> just commented out ;-) I fixed a typo and added some comments.
>
> Bruce suggested that this should get some code review, since a bug in
> pg_dump would be bad news. Any comments would be welcome.
>
> Cheers,
>
> Neil
>
> --
> Neil Conway <neilconway@rogers.com>
> PGP Key ID: DB3C29FC
>


Tue, 19 Feb 2002 11:44:06 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
Re: pg_dump: use ALTER TABLE for PKs
Neil Conway writes:

> I've attached a patch which enhances pg_dump to use ALTER TABLE when
> defining the primary keys of a table (instead of including this
> information in the table definition).

I think you've got the "7.2 or later" theme in your patch mixed up.  The
version number you're evaluating is the version of the database you're
dumping.  The feature you're adding is dependent on the version of the
database you're restoring into.  The version of the database you're
restoring into is always >= the pg_dump version.  Thus, you don't need any
version checks.  Plus, I think this patch is going into 7.3, no?

-- 
Peter Eisentraut   peter_e@gmx.net


Mon, 18 Feb 2002 22:46:12 -0500 (EST)
Peter Eisentraut <peter_e@gmx.net>
Re: pg_dump: use ALTER TABLE for PKs
On Mon, 2002-02-18 at 22:46, Peter Eisentraut wrote:
> Neil Conway writes:
> 
> > I've attached a patch which enhances pg_dump to use ALTER TABLE when
> > defining the primary keys of a table (instead of including this
> > information in the table definition).
> 
> I think you've got the "7.2 or later" theme in your patch mixed up.  The
> version number you're evaluating is the version of the database you're
> dumping.  The feature you're adding is dependent on the version of the
> database you're restoring into.  The version of the database you're
> restoring into is always >= the pg_dump version.  Thus, you don't need any
> version checks.

You're absolutely correct -- thanks for spotting that. A new version of
the patch is attached.

Cheers,

Neil

-- 
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC
18 Feb 2002 23:15:05 -0500
Neil Conway <nconway@klamath.dyndns.org>
Re: pg_dump: use ALTER TABLE for PKs
At 21:26 18/02/02 -0500, Neil Conway wrote:
>The patch is pretty simple, partly because the code was already there,
>just commented out ;-) I fixed a typo and added some comments.

When I originally wrote the commented-out code, we did not have the ability
to make 'dependant' TOC Entries, so it uses the OID of the index as the PK
OID in the ArchiveEntry call. 

I can't really think of a case where the index OID would be less than the
table OID, but it might be good to put the table OID in the deps list; this
will force the PK to go after the table in all cases. Did I mention I'm
paranoid?

It's also a first step in the ability to dump a 'whole table' Vs. just a
table definition - which we will need when all constraints can be dumped
via 'ALTER TABLE'.

For clarity, 'whole table' might mean the attrs, indexes, constraints,
triggers etc.


----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 0500 83 82 82         |                 ___________ |
Http://www.rhyme.com.au          |                /           \|
                                 |    --________--
PGP key available upon request,  |  /
and from pgp5.ai.mit.edu:11371   |/

Wed, 20 Feb 2002 01:23:35 +1100
Philip Warner <pjw@rhyme.com.au>
Re: pg_dump: use ALTER TABLE for PKs
>
>For clarity, 'whole table' might mean the attrs, indexes, constraints,
>triggers etc.
>

Sorry, this should probably be limited to anything that can go into a
'CREATE TABLE' statement, so triggers & indexes would not be included - but
constraints, default values etc would be there.


----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 0500 83 82 82         |                 ___________ |
Http://www.rhyme.com.au          |                /           \|
                                 |    --________--
PGP key available upon request,  |  /
and from pgp5.ai.mit.edu:11371   |/

Wed, 20 Feb 2002 01:41:04 +1100
Philip Warner <pjw@rhyme.com.au>
Re: pg_dump: use ALTER TABLE for PKs
Philip Warner <pjw@rhyme.com.au> writes:
> I can't really think of a case where the index OID would be less than the
> table OID, but it might be good to put the table OID in the deps list; this
> will force the PK to go after the table in all cases. Did I mention I'm
> paranoid?

After OID-counter wraparound it'd be quite easy to have such a case.

To the extent that pg_dump is depending on OID comparison to tell it
something about creation order, it's really broken and should be fixed.
The dependencies mechanism should eventually be extended to cover all
these relationships.

			regards, tom lane

Tue, 19 Feb 2002 10:09:10 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pg_dump: use ALTER TABLE for PKs 
At 21:26 18/02/02 -0500, Neil Conway wrote:
>
>Bruce suggested that this should get some code review, since a bug in
>pg_dump would be bad news. Any comments would be welcome.
>

Part of the reason I did not implement this earlier was that ALTER
TABLE...PK did not handle child tables 'properly' (for some arbitrary
meaning of the word 'properly'). How have you covered:

    CREATE TABLE PARENT(F1 INT PRIMARY KEY);
    CREATE TABLE CHILD(...) INHERIT PARENT

this should create a PK on CHILD; what does pg-dump and the ALTER TABLE
implementation do? Not sure how it should work, but ultimately we need to
put the PK (and, necessarily, FK) creation at the end of the data load:

    CREATE TABLE PARENT(F1 INT);
    CREATE TABLE CHILD(...) INHERIT PARENT

    Load PARENT
    Load CHILD

    ALTER TABLE PARENT...PK
    ALTER TABLE CHILD...PK

This is a non-trivial issue since we also need to cater for:

    CREATE TABLE PARENT(F1 INT);
    CREATE TABLE CHILD1(...) INHERIT PARENT
    ALTER TABLE PARENT...PK
    CREATE TABLE CHILD2(...) INHERIT PARENT

Which, at least historically, resulted in CHILD1 *not* having a PK. I would
have built CVS and checked myself, but I'm relocating office (and servers)
at the moment.


----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 0500 83 82 82         |                 ___________ |
Http://www.rhyme.com.au          |                /           \|
                                 |    --________--
PGP key available upon request,  |  /
and from pgp5.ai.mit.edu:11371   |/

Wed, 20 Feb 2002 07:53:04 +1100
Philip Warner <pjw@rhyme.com.au>
Re: pg_dump: use ALTER TABLE for PKs

On Wed, 20 Feb 2002, Philip Warner wrote:

> At 21:26 18/02/02 -0500, Neil Conway wrote:
> >
> >Bruce suggested that this should get some code review, since a bug in
> >pg_dump would be bad news. Any comments would be welcome.
> >
>
> Part of the reason I did not implement this earlier was that ALTER
> TABLE...PK did not handle child tables 'properly' (for some arbitrary
> meaning of the word 'properly'). How have you covered:
>
>     CREATE TABLE PARENT(F1 INT PRIMARY KEY);
>     CREATE TABLE CHILD(...) INHERIT PARENT
>
> this should create a PK on CHILD; what does pg-dump and the ALTER TABLE
> implementation do? Not sure how it should work, but ultimately we need to

Unless it was changed between rc2 and release, the above type of
sequence does not end up with a primary key on child(f1).



Tue, 19 Feb 2002 13:07:55 -0800 (PST)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
Re: pg_dump: use ALTER TABLE for PKs
At 13:07 19/02/02 -0800, Stephan Szabo wrote:
>>
>>     CREATE TABLE PARENT(F1 INT PRIMARY KEY);
>>     CREATE TABLE CHILD(...) INHERIT PARENT
>>
>> this should create a PK on CHILD; what does pg-dump and the ALTER TABLE
>> implementation do? Not sure how it should work, but ultimately we need to
>
>Unless it was changed between rc2 and release, the above type of
>sequence does not end up with a primary key on child(f1).

Interesting, - that makes pg_dumps job easier. Are any constraints ever
inherited?


----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 0500 83 82 82         |                 ___________ |
Http://www.rhyme.com.au          |                /           \|
                                 |    --________--
PGP key available upon request,  |  /
and from pgp5.ai.mit.edu:11371   |/

Wed, 20 Feb 2002 08:37:46 +1100
Philip Warner <pjw@rhyme.com.au>
Re: pg_dump: use ALTER TABLE for PKs

On Wed, 20 Feb 2002, Philip Warner wrote:

> At 13:07 19/02/02 -0800, Stephan Szabo wrote:
> >>
> >>     CREATE TABLE PARENT(F1 INT PRIMARY KEY);
> >>     CREATE TABLE CHILD(...) INHERIT PARENT
> >>
> >> this should create a PK on CHILD; what does pg-dump and the ALTER TABLE
> >> implementation do? Not sure how it should work, but ultimately we need to
> >
> >Unless it was changed between rc2 and release, the above type of
> >sequence does not end up with a primary key on child(f1).
>
> Interesting, - that makes pg_dumps job easier. Are any constraints ever
> inherited?

Check constraints and not null I believe are inherited by default.
But I just thought of a case that won't dump and restore the same as it
was originally made because we support ONLY on alter table add constraint.

create table z(a int);
create table z2() inherits(z);
alter table only z add constraint foo1 check(a>4);

will make z have a constraint on a but not z2 and the check will get
dumped as part of z's definition which will restore with z2 having
the check.


Tue, 19 Feb 2002 14:05:56 -0800 (PST)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
Re: pg_dump: use ALTER TABLE for PKs
At 14:05 19/02/02 -0800, Stephan Szabo wrote:
>
>Check constraints and not null I believe are inherited by default.
>But I just thought of a case that won't dump and restore the same as it
>was originally made because we support ONLY on alter table add constraint.
>
>create table z(a int);
>create table z2() inherits(z);
>alter table only z add constraint foo1 check(a>4);
>
>will make z have a constraint on a but not z2 and the check will get
>dumped as part of z's definition which will restore with z2 having
>the check.

This brings to mind a couple of questions:

1. Do you have plans to support inhertiance of PK constraints?
2. Does/will DROP CONSTRAINT support ONLY?


----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 0500 83 82 82         |                 ___________ |
Http://www.rhyme.com.au          |                /           \|
                                 |    --________--
PGP key available upon request,  |  /
and from pgp5.ai.mit.edu:11371   |/

Wed, 20 Feb 2002 10:11:59 +1100
Philip Warner <pjw@rhyme.com.au>
Re: pg_dump: use ALTER TABLE for PKs
The behaviour you illustrate below does not exist and therefore is not a
problem:

test=# create table parent(f1 int primary key);
NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index 'parent_pkey'
for t
able 'parent'
CREATE
test=# create table child(f2 int) inherits (parent);
CREATE
test=# \d parent
        Table "parent"
 Column |  Type   | Modifiers
--------+---------+-----------
 f1     | integer | not null
Primary key: parent_pkey

test=# \d child
        Table "child"
 Column |  Type   | Modifiers
--------+---------+-----------
 f1     | integer | not null
 f2     | integer |


Chris

> -----Original Message-----
> From: Philip Warner [mailto:pjw@rhyme.com.au]
> Sent: Wednesday, 20 February 2002 4:53 AM
> To: Neil Conway; pgsql-hackers@postgresql.org
> Cc: Christopher Kings-Lynne
> Subject: Re: [HACKERS] pg_dump: use ALTER TABLE for PKs
>
>
> At 21:26 18/02/02 -0500, Neil Conway wrote:
> >
> >Bruce suggested that this should get some code review, since a bug in
> >pg_dump would be bad news. Any comments would be welcome.
> >
>
> Part of the reason I did not implement this earlier was that ALTER
> TABLE...PK did not handle child tables 'properly' (for some arbitrary
> meaning of the word 'properly'). How have you covered:
>
>     CREATE TABLE PARENT(F1 INT PRIMARY KEY);
>     CREATE TABLE CHILD(...) INHERIT PARENT
>
> this should create a PK on CHILD; what does pg-dump and the ALTER TABLE
> implementation do? Not sure how it should work, but ultimately we need to
> put the PK (and, necessarily, FK) creation at the end of the data load:
>
>     CREATE TABLE PARENT(F1 INT);
>     CREATE TABLE CHILD(...) INHERIT PARENT
>
>     Load PARENT
>     Load CHILD
>
>     ALTER TABLE PARENT...PK
>     ALTER TABLE CHILD...PK
>
> This is a non-trivial issue since we also need to cater for:
>
>     CREATE TABLE PARENT(F1 INT);
>     CREATE TABLE CHILD1(...) INHERIT PARENT
>     ALTER TABLE PARENT...PK
>     CREATE TABLE CHILD2(...) INHERIT PARENT
>
> Which, at least historically, resulted in CHILD1 *not* having a
> PK. I would
> have built CVS and checked myself, but I'm relocating office (and servers)
> at the moment.
>
>
> ----------------------------------------------------------------
> Philip Warner                    |     __---_____
> Albatross Consulting Pty. Ltd.   |----/       -  \
> (A.B.N. 75 008 659 498)          |          /(@)   ______---_
> Tel: (+61) 0500 83 82 81         |                 _________  \
> Fax: (+61) 0500 83 82 82         |                 ___________ |
> Http://www.rhyme.com.au          |                /           \|
>                                  |    --________--
> PGP key available upon request,  |  /
> and from pgp5.ai.mit.edu:11371   |/
>


Wed, 20 Feb 2002 10:53:57 +0800
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
Re: pg_dump: use ALTER TABLE for PKs

On Wed, 20 Feb 2002, Philip Warner wrote:

> At 14:05 19/02/02 -0800, Stephan Szabo wrote:
> >
> >Check constraints and not null I believe are inherited by default.
> >But I just thought of a case that won't dump and restore the same as it
> >was originally made because we support ONLY on alter table add constraint.
> >
> >create table z(a int);
> >create table z2() inherits(z);
> >alter table only z add constraint foo1 check(a>4);
> >
> >will make z have a constraint on a but not z2 and the check will get
> >dumped as part of z's definition which will restore with z2 having
> >the check.
>
> This brings to mind a couple of questions:
>
> 1. Do you have plans to support inhertiance of PK constraints?
If you mean me personally, not in the short term.  I don't think
that pk over an inheritance tree means a copy of the constraint
for each table either, so it's not quite the same as some of the
other constraints.

> 2. Does/will DROP CONSTRAINT support ONLY?
Pretty sure it does.  I also think it'll let you drop an inherited
constraint from a child table which can get you into the same state as
the create/alter above.  Does dropping inherited constraints from
children or allowing constraints to be added to only the parent table
make sense? I only wonder because unless you're using only on the queries
you'll see the rows in the children that violate the constraint which
seems kinda odd to me.


Wed, 20 Feb 2002 08:02:53 -0800 (PST)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
Re: pg_dump: use ALTER TABLE for PKs

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 Mon, 2002-02-18 at 22:46, Peter Eisentraut wrote:
> > Neil Conway writes:
> > 
> > > I've attached a patch which enhances pg_dump to use ALTER TABLE when
> > > defining the primary keys of a table (instead of including this
> > > information in the table definition).
> > 
> > I think you've got the "7.2 or later" theme in your patch mixed up.  The
> > version number you're evaluating is the version of the database you're
> > dumping.  The feature you're adding is dependent on the version of the
> > database you're restoring into.  The version of the database you're
> > restoring into is always >= the pg_dump version.  Thus, you don't need any
> > version checks.
> 
> You're absolutely correct -- thanks for spotting that. A new version of
> the patch is attached.
> 
> Cheers,
> 
> Neil
> 
> -- 
> Neil Conway <neilconway@rogers.com>
> PGP Key ID: DB3C29FC

[ Attachment, skipping... ]

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  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

Fri, 22 Feb 2002 21:36:56 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pg_dump: use ALTER TABLE for PKs
One persons view, unfortunately probably shared by others

http://www.open-mag.com/42444203327.htm

Dave


Sat, 23 Feb 2002 10:00:06 -0500
"Dave Cramer" <Dave@micro-automation.net>
Open magazine article on open source rdbms
> One persons view, unfortunately probably shared by others
> http://www.open-mag.com/42444203327.htm

Hmm. He has a few facts right imho; the last few sentences of the
article describe exactly how PostgreSQL *does* get adopted. Actual
adoption and use will precede trade press reports by several years (c.f.
Linux adoption and severely lagging trade press recognition of such).

Doesn't bother me a bit, and I am always amused by the bandwagoner
journalists changing their stories once the cat is out of the bag. It'll
happen here too ;)

                      - Thomas

Mon, 25 Feb 2002 07:05:20 -0800
Thomas Lockhart <thomas@fourpalms.org>
Re: Open magazine article on open source rdbms
Dave Cramer wrote:
> One persons view, unfortunately probably shared by others
> 
> http://www.open-mag.com/42444203327.htm

Yea, I shot this over to the general list.  I found it an interesting
read, and may be true for very large customers.  He is correct that we
are more popular with smaller customers.  No question about that.

-- 
  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

Mon, 25 Feb 2002 11:33:42 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Open magazine article on open source rdbms
> Hmm. He has a few facts right imho; the last few sentences of the
> article describe exactly how PostgreSQL *does* get adopted. Actual
> adoption and use will precede trade press reports by several years (c.f.
> Linux adoption and severely lagging trade press recognition of such).
> 
> Doesn't bother me a bit, and I am always amused by the bandwagoner
> journalists changing their stories once the cat is out of the bag. It'll
> happen here too ;)

Yes, Linux is an excellent parallel.

-- 
  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

Mon, 25 Feb 2002 13:31:16 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Open magazine article on open source rdbms