Re: ALTER TABLE...ALTER COLUMN vs inheritance

Bernd Helmle <mailings@oopsware.de>

From: Bernd Helmle <mailings@oopsware.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Date: 2010-05-26T21:37:35Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Catalog NOT NULL constraints

Attachments


--On 4. November 2009 09:57:27 -0500 Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Yeah, this is a known issue.  The ALTER should be rejected, but it is
> not, because we don't have enough infrastructure to notice that the
> constraint is inherited and logically can't be dropped.  I think the
> consensus was that the way to fix this (along with some other problems)
> is to start representing NOT NULL constraints in pg_constraint, turning
> attnotnull into just a bit of denormalization for performance.

Lost it a little from my radar, but here's is a first shot on this issue 
now. The patch creates a new CONSTRAINT_NOTNULL contype and assigns all 
required information for the NOT NULL constraint to it. Currently the 
constraint records the attribute number it belongs to and manages the 
inheritance properties. Passes regression tests with some adjustments to 
pg_constraint output.

The patch as it stands employs a dedicated code path for 
ATExecDropNotNull(), thus duplicates the behavior of 
ATExecDropConstraint(). I'm not really satisfied with this, but i did it 
this way to prevent some heavy conditional rearrangement in 
ATExecDropConstraint(). Maybe its worth to move the code to adjust 
constraint inheritance properties into a separate function.

There's also a remaining issue which needs to be addressed: currently 
pg_get_constraintdef_worker() is special case'd to dump the correct syntax 
for the NOT NULL constraint, which is totally redundant. I'm not sure how 
to do it correctly, since for example ATExecAlterColumnType() actually uses 
it to restore dependent constraints on a column. We might want to just move 
the special case there.

I understand that people are busy with the remaining open items list for 
9.0, so it's okay to discuss this during the upcoming reviewfest.


        Bernd