DROP CONSTRAINT patch

Anyone looked at this yet?

Also, if someone could tell me where I should attempt to add a regression
test and what, exactly, I should be regression testing it would be
helpful...

Chris

Anyone looked at this yet?

Also, if someone could tell me where I should attempt to add a regression
test and what, exactly, I should be regression testing it would be
helpful...

I was hoping someone else would comment on it. I will check it out
later and either apply or comment. You seemed little uncertain of it so
I left it in my mailbox for a while.

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

No actually, I'm quite confident of it - it works perfectly on my computer
and does exactly what it's supposed to do. I have tested it extensively.
It also works in a transaction and gets rolled back properly. However, I
haven't tested it with multiple backends simultaneously doing stuff. I'm
pretty sure there won't be anything wrong with it though...

It's the stylistic stuff I don't know about. For example, I issue a NOTICE
if more than one constraint was removed - I don't know if this is desirable
behaviour from the viewpoint of you guys (I personally think it's very
important!).

But hey...

Chris

On Tue, 22 May 2001, Christopher Kings-Lynne wrote:

Anyone looked at this yet?

Also, if someone could tell me where I should attempt to add a regression
test and what, exactly, I should be regression testing it would be
helpful...

At the risk of making it even longer, probably alter_table.sql.
You probably want to try out various conceivable uses of the drop
constraint, including error conditions.

Some things like:
create table with constraint
try to insert valid row
try to insert invalid row
drop the constraint
try to insert valid row
try to insert row that was invalid

create table with two equal named constraints
insert valid to both
insert valid to one but not two
insert valid to two but not one
insert valid to neither
...

create table with two non-equal named constraints
(do inserts)
drop constraint one
try to insert valid for both,
valid for one but not two
valid for two but not one
valid for neither
drop constraint two
(do more inserts)

For the add/drop constraint clauses would it be an idea to change the syntax
to:

ALTER TABLE [ ONLY ] x ADD CONSTRAINT x;
ALTER TABLE [ ONLY ] x DROP CONSTRAINT x;

So that people can specify whether the constraint should be inherited or
not?

Chris

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 withing the next 48 hours.

Hi all,

Attached is my patch that adds DROP CONSTRAINT support to PostgreSQL. I
basically want your guys feedback. I have sprinkled some of my q's thru
the text delimited with the @@ symbol. It seems to work perfectly.

At the moment it does CHECK constraints only, with inheritance. However,
due to the problem mentioned before with the mismatching between inherited
constraints it may be wise to disable the inheritance feature for a while.
it is written in an extensible fashion to support future dropping of other
types of constraint, and is well documented.

Please send me your comments, check my use of locking, updating of
indices, use of ERROR and NOTICE, etc. and I will rework the patch based
on feedback until everyone
is happy with it...

Chris

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://www.postgresql.org/search.mpl

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

On Wed, 23 May 2001, Christopher Kings-Lynne wrote:

For the add/drop constraint clauses would it be an idea to change the syntax
to:

ALTER TABLE [ ONLY ] x ADD CONSTRAINT x;
ALTER TABLE [ ONLY ] x DROP CONSTRAINT x;

So that people can specify whether the constraint should be inherited or
not?

I'm not sure. I don't much use the inheritance stuff, so I'm not sure
what would be better for them. Hopefully one of the people interested
in inheritance will speak up. :) Technically it's probably not difficult
to do.

A related question is whether or not you can drop a constraint on a
subtable that's inherited from a parent.

Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:

On Wed, 23 May 2001, Christopher Kings-Lynne wrote:

For the add/drop constraint clauses would it be an idea to change the syntax
to:

ALTER TABLE [ ONLY ] x ADD CONSTRAINT x;
ALTER TABLE [ ONLY ] x DROP CONSTRAINT x;

If the patch is coded in the same style as the existing ALTER code then
this will happen automatically. Are you looking at current development
tip as the comparison point for your changes?

A related question is whether or not you can drop a constraint on a
subtable that's inherited from a parent.

There is the question of whether it's a good idea to allow a constraint
to exist on a parent but not on its subtables. Seems like a bad idea to
me. But as long as the default is to propagate these changes, I'm not
really eager to prohibit DBAs from doing the other. Who's to say what's
a misuse of inheritance and what's not...

regards, tom lane

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

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

Would you like me to submit a version minus all my questions? And what
about the inheritance feature - should it be left enabled (as is) at the
moment?

Chris

Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:

On Wed, 23 May 2001, Christopher Kings-Lynne wrote:

For the add/drop constraint clauses would it be an idea to

change the syntax

to:

ALTER TABLE [ ONLY ] x ADD CONSTRAINT x;
ALTER TABLE [ ONLY ] x DROP CONSTRAINT x;

If the patch is coded in the same style as the existing ALTER code then
this will happen automatically. Are you looking at current development
tip as the comparison point for your changes?

I'm not sure what you mean here, Tom - I meant that the ONLY keyword could
be optional. I know that most of the existing ADD CONSTRAINT code does not
propagate constraints to children. However, if it was ever changed so that
it did - would it be nice to allow the DBA to specify that it should not be
propagated.

A related question is whether or not you can drop a constraint on a
subtable that's inherited from a parent.

There is the question of whether it's a good idea to allow a constraint
to exist on a parent but not on its subtables.

It seems to me that someone needs to sit down and decide on the inheritance
semantics that should be enforced (ideally) and then they can be coded to
the design.

Seems like a bad idea to
me. But as long as the default is to propagate these changes, I'm not
really eager to prohibit DBAs from doing the other. Who's to say what's
a misuse of inheritance and what's not...

At the moment we have:

* ADD CONSTRAINT does not propagate
* If you create a table with a CHECK constraint, then create a table that
inherits from that, the CHECK constraint _does_ propagate.

Seems to me that these behaviours are inconsistent...

Chris

"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:

I'm not sure what you mean here, Tom - I meant that the ONLY keyword could
be optional.

The current gram.y code allows either ALTER TABLE foo ONLY or ALTER
TABLE foo* for all forms of ALTER ... with the default interpretation
being the latter.

At the moment we have:
* ADD CONSTRAINT does not propagate

I doubt you will find anyone who's willing to argue that that's not a
bug --- specifically, AlterTableAddConstraint()'s lack of inheritance
recursion like its siblings have. Feel free to fix it.

regards, tom lane

"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:

I'm not sure what you mean here, Tom - I meant that the ONLY

keyword could

be optional.

The current gram.y code allows either ALTER TABLE foo ONLY or ALTER
TABLE foo* for all forms of ALTER ... with the default interpretation
being the latter.

Oops - ok, I didn't notice that...hmmm...maybe I should check my patch for
that before Bruce commits it...

At the moment we have:
* ADD CONSTRAINT does not propagate

I doubt you will find anyone who's willing to argue that that's not a
bug --- specifically, AlterTableAddConstraint()'s lack of inheritance
recursion like its siblings have. Feel free to fix it.

It was next on my list. However, I don't want to step on Stephan's toes...

Chris

[ Charset ISO-8859-1 unsupported, converting... ]

"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:

I'm not sure what you mean here, Tom - I meant that the ONLY

keyword could

be optional.

The current gram.y code allows either ALTER TABLE foo ONLY or ALTER
TABLE foo* for all forms of ALTER ... with the default interpretation
being the latter.

Oops - ok, I didn't notice that...hmmm...maybe I should check my patch for
that before Bruce commits it...

Got it. Patch on hold.

As you can see, inheritance really needs some attention. Feel free to
give it.

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

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

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

Would you like me to submit a version minus all my questions? And what
about the inheritance feature - should it be left enabled (as is) at the
moment?

You can send me a totally new patch if you wish. That way, you can
throw the whole patch to the patches list for comment and improvement.

If you would prefer for me to apply it in pieces, I can.

Please keep going and resolve these inheritance issues.

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

You can send me a totally new patch if you wish. That way, you can
throw the whole patch to the patches list for comment and improvement.

I'll do that.

Please keep going and resolve these inheritance issues.

It's not that there's inheritance issues in my patch. I am actually certain
that it will work fine with the ONLY keyword. However, I will make some
changes and submit it to pgsql-patches.

The problem is that although my patch can quite happily remove all
constraints of the same name from all the child tables, the problem is - can
we be sure that all the constraints with that name are actually inherited
from the parent constraint. Answer - no. At least, not until the add
constraint stuff is all fixed. So maybe I will just hash out inheritance
support for the time being.

Chris

You can send me a totally new patch if you wish. That way, you can
throw the whole patch to the patches list for comment and improvement.

I'll do that.

Please keep going and resolve these inheritance issues.

It's not that there's inheritance issues in my patch. I am actually certain
that it will work fine with the ONLY keyword. However, I will make some
changes and submit it to pgsql-patches.

The problem is that although my patch can quite happily remove all
constraints of the same name from all the child tables, the problem is - can
we be sure that all the constraints with that name are actually inherited
from the parent constraint. Answer - no. At least, not until the add
constraint stuff is all fixed. So maybe I will just hash out inheritance
support for the time being.

Oh, I was hoping you could at least list _all_ the inheritance issues
for the TODO list. Seems you have found some already. If we have that,
we can get a plan for someone to finally fix them all.

What we really need is a list, and a discussion, and decisions, rather
than just letting it lay around.

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

Seems like a bad idea to
me. But as long as the default is to propagate these changes, I'm not
really eager to prohibit DBAs from doing the other. Who's to say what's
a misuse of inheritance and what's not...

At the moment we have:

* ADD CONSTRAINT does not propagate
* If you create a table with a CHECK constraint, then create a table that
inherits from that, the CHECK constraint _does_ propagate.

Seems to me that these behaviours are inconsistent...

Yep, but I've got the minimal patch to fix ADD CONSTRAINT. I'm just
waiting for the upcoming weekend so I can add the regression tests and
pack it up.

Thanks. Patch applied. @@ comments removed because patch was reviewed.

Hi all,

Attached is my patch that adds DROP CONSTRAINT support to PostgreSQL. I
basically want your guys feedback. I have sprinkled some of my q's thru
the text delimited with the @@ symbol. It seems to work perfectly.

At the moment it does CHECK constraints only, with inheritance. However,
due to the problem mentioned before with the mismatching between inherited
constraints it may be wise to disable the inheritance feature for a while.
it is written in an extensible fashion to support future dropping of other
types of constraint, and is well documented.

Please send me your comments, check my use of locking, updating of
indices, use of ERROR and NOTICE, etc. and I will rework the patch based
on feedback until everyone
is happy with it...

Chris

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://www.postgresql.org/search.mpl

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

Thanks. Patch applied. @@ comments removed because patch was reviewed.

Actually, can someone make sure all the @@ comments have been dealt
with? I assume people checked them, but maybe not.

Hi all,

Attached is my patch that adds DROP CONSTRAINT support to PostgreSQL. I
basically want your guys feedback. I have sprinkled some of my q's thru
the text delimited with the @@ symbol. It seems to work perfectly.

At the moment it does CHECK constraints only, with inheritance. However,
due to the problem mentioned before with the mismatching between inherited
constraints it may be wise to disable the inheritance feature for a while.
it is written in an extensible fashion to support future dropping of other
types of constraint, and is well documented.

Please send me your comments, check my use of locking, updating of
indices, use of ERROR and NOTICE, etc. and I will rework the patch based
on feedback until everyone
is happy with it...

Chris

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://www.postgresql.org/search.mpl

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

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go 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