Thread: pg_dump: use ALTER TABLE for PKs

pg_dump: use ALTER TABLE for PKs

From
Neil Conway
Date:
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

Attachment

Re: pg_dump: use ALTER TABLE for PKs

From
Neil Conway
Date:
On Mon, 2002-02-18 at 22:24, Bruce Momjian wrote:
> 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.
> 
> The issue was that the author wants it in 7.2.1.

I'm confused: which author? Where does this information come from?

>  I question whether a
> purely performance pg_dump patch belongs in a minor release.  I will let
> others decide.

I agree.

Cheers,

Neil

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



Re: pg_dump: use ALTER TABLE for PKs

From
Neil Conway
Date:
On Mon, 2002-02-18 at 22:35, Bruce Momjian wrote:
> Neil Conway wrote:
> > I'm confused: which author? Where does this information come from?
> 
> I thought you were the author.

I am ;-)

> Oh, I thought you wanted this in 7.2.1.

Miscommunication, I think -- in a private email to you, I wrote "I don't
think this is appropriate for 7.2.1, in any case". ;-)

> it just needs the normal review, not extra special review.  :-)

Okay.

Cheers,

Neil

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



Re: pg_dump: use ALTER TABLE for PKs

From
"Christopher Kings-Lynne"
Date:
> > The issue was that the author wants it in 7.2.1.
>
> I'm confused: which author? Where does this information come from?

I certainly didn't propose that it appear in 7.2.1 - I was thinking 7.3.

> >  I question whether a
> > purely performance pg_dump patch belongs in a minor release.  I will let
> > others decide.

It sure shouldn't...

Chris



Re: pg_dump: use ALTER TABLE for PKs

From
Peter Eisentraut
Date:
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



Re: pg_dump: use ALTER TABLE for PKs

From
"Christopher Kings-Lynne"
Date:
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
>



Re: pg_dump: use ALTER TABLE for PKs

From
Neil Conway
Date:
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

Re: pg_dump: use ALTER TABLE for PKs

From
Philip Warner
Date:
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   |/


Re: pg_dump: use ALTER TABLE for PKs

From
Philip Warner
Date:
>
>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   |/


Re: pg_dump: use ALTER TABLE for PKs

From
Tom Lane
Date:
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


Re: pg_dump: use ALTER TABLE for PKs

From
Philip Warner
Date:
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   |/


Re: pg_dump: use ALTER TABLE for PKs

From
Stephan Szabo
Date:
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).




Re: pg_dump: use ALTER TABLE for PKs

From
Philip Warner
Date:
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   |/


Re: pg_dump: use ALTER TABLE for PKs

From
Stephan Szabo
Date:
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.



Re: pg_dump: use ALTER TABLE for PKs

From
Philip Warner
Date:
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   |/


Re: pg_dump: use ALTER TABLE for PKs

From
"Christopher Kings-Lynne"
Date:
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 nullf2     | 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   |/
>



Re: pg_dump: use ALTER TABLE for PKs

From
Stephan Szabo
Date:
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.



Re: pg_dump: use ALTER TABLE for PKs

From
Bruce Momjian
Date:
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,
Pennsylvania19026
 


Open magazine article on open source rdbms

From
"Dave Cramer"
Date:
One persons view, unfortunately probably shared by others

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

Dave



Re: Open magazine article on open source rdbms

From
Thomas Lockhart
Date:
> 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


Re: Open magazine article on open source rdbms

From
Bruce Momjian
Date:
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,
Pennsylvania19026
 


Re: Open magazine article on open source rdbms

From
Bruce Momjian
Date:
> 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,
Pennsylvania19026