Patch: Extend NOT NULL representation to pg_constraint

Hi all.

My name is Jose Arthur and I use PostgreSQL for a while, but never
contributed to the main project, until now.

Since nobody else take this patch to review in this commitfest, I'm going to
try :-). The main problem can be found here:
http://archives.postgresql.org/pgsql-hackers/2009-11/msg00146.php

How to simulate:

CREATE TABLE foo(id integer NOT NULL, val text NOT NULL);
CREATE TABLE foo2(another_id integer NOT NULL) INHERITS(foo);
ALTER TABLE foo2 ALTER COLUMN val DROP NOT NULL;
insert into foo2 values (1, null, 1);

Then pg_dump > dump.pgsql and psql -f dump.pgsql

I've applied the patch submitted here:
http://archives.postgresql.org/pgsql-hackers/2010-09/msg00763.php against
current master (635de8365f0299cfa2db24c56abcfccb65d020f0) and compile is
fine

Using the patch, I can't drop NOT NULL from foo2, just from foo, and I think
that makes sense:

postgres=# ALTER TABLE foo2 ALTER COLUMN val DROP NOT NULL;
ERROR: cannot drop inherited NOT NULL constraint "foo2_val_not_null",
relation "foo2"

One thing that I take notice is when you create a simple table like this
one: select count(*) from pg_constraint ; 12 rows appears in pg_constraint,
10 to the sequence. Is that ok?

Other thing:
#define CONSTRAINT_NOTNULL 'n' in
src/include/catalog/pg_constraint.h is using spaces instead of tabs :-)

--
José Arthur Benetasso Villanova

--On 25. September 2010 19:55:02 -0300 José Arthur Benetasso Villanova
<jose(dot)arthur(at)gmail(dot)com> wrote:

> One thing that I take notice is when you create a simple table like this
> one: select count(*) from pg_constraint ; 12 rows appears in
> pg_constraint, 10 to the sequence. Is that ok?

Not sure i get you here, can you elaborate more on this?

--
Thanks

Bernd

On Sun, Sep 26, 2010 at 2:11 PM, Bernd Helmle <mailings(at)oopsware(dot)de> wrote:
> --On 25. September 2010 19:55:02 -0300 José Arthur Benetasso Villanova
> <jose(dot)arthur(at)gmail(dot)com> wrote:
>
>> One thing that I take notice is when you create a simple table like this
>> one: select count(*) from pg_constraint ; 12 rows appears in
>> pg_constraint, 10 to the sequence. Is that ok?
>
> Not sure i get you here, can you elaborate more on this?

I think his question was - how do we feel about the massive catalog
bloat this patch will create?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I think his question was - how do we feel about the massive catalog
> bloat this patch will create?

It's a fair question.

I can imagine designing things so that we don't create an explicit
pg_constraint row for the simplest case of an unnamed, non-inherited
NOT NULL constraint. Seems like it would complicate matters quite
a lot though, in exchange for saving what in the end isn't an enormous
amount of space.

regards, tom lane

--On 26. September 2010 15:50:06 -0400 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

>> I think his question was - how do we feel about the massive catalog
>> bloat this patch will create?
>
> It's a fair question.
>
> I can imagine designing things so that we don't create an explicit
> pg_constraint row for the simplest case of an unnamed, non-inherited
> NOT NULL constraint. Seems like it would complicate matters quite
> a lot though, in exchange for saving what in the end isn't an enormous
> amount of space.

What i can try is to record the inheritance information only in case of
attinhcount > 0. This would make maintenance of the pg_constraint records
for NOT NULL columns a little complicater though. Another thing we should
consider is that Peter's functional dependency patch was supposed to rely
on this feature (1), once it gets done. Not sure this still holds true....

1)
<http://archives.postgresql.org/message-id/1279361718.17928.1.camel@vanquo.pezone.net>

--
Thanks

Bernd

Bernd Helmle <mailings(at)oopsware(dot)de> writes:
> What i can try is to record the inheritance information only in case of
> attinhcount > 0. This would make maintenance of the pg_constraint records
> for NOT NULL columns a little complicater though. Another thing we should
> consider is that Peter's functional dependency patch was supposed to rely
> on this feature (1), once it gets done. Not sure this still holds true....

Oh, right, that's a killer argument. Finishing that patch still
requires that NOT NULL constraints have pg_constraint OIDs assigned,
which means they *have to* have pg_constraint rows to carry the OIDs.
So forget the whole thing; we'll just eat the space penalty.

regards, tom lane