Thread: Patch: Extend NOT NULL representation to pg_constraint

Patch: Extend NOT NULL representation to pg_constraint

From
José Arthur Benetasso Villanova
Date:
Hi all.<br /><br />My name is Jose Arthur and I use PostgreSQL for a while, but never contributed to the main project,
untilnow.<br /><br />Since nobody else take this patch to review in this commitfest, I'm going to try :-). The main
problemcan be found here: <a
href="http://archives.postgresql.org/pgsql-hackers/2009-11/msg00146.php">http://archives.postgresql.org/pgsql-hackers/2009-11/msg00146.php</a><br
clear="all"/><br />How to simulate:<br /><br />CREATE TABLE foo(id integer NOT NULL, val text NOT NULL);<br />CREATE
TABLEfoo2(another_id integer NOT NULL) INHERITS(foo);<br />ALTER TABLE foo2 ALTER COLUMN val DROP NOT NULL;<br />insert
intofoo2 values (1, null, 1);<br /><br />Then pg_dump > dump.pgsql and psql -f dump.pgsql<br /><br />I've applied
thepatch submitted here: <a
href="http://archives.postgresql.org/pgsql-hackers/2010-09/msg00763.php">http://archives.postgresql.org/pgsql-hackers/2010-09/msg00763.php</a>
againstcurrent master (635de8365f0299cfa2db24c56abcfccb65d020f0) and compile is fine<br /><br />Using the patch, I
can'tdrop NOT NULL from foo2, just from foo, and I think that makes sense:<br /><br />postgres=# ALTER TABLE foo2 ALTER
COLUMNval DROP NOT NULL;<br />ERROR:  cannot drop inherited NOT NULL constraint "foo2_val_not_null", relation "foo2"<br
/><br/>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?<br /><br />Other thing: <br />#define
CONSTRAINT_NOTNULL         'n' in src/include/catalog/pg_constraint.h is using spaces instead of tabs :-)<br /><br
/><br/>-- <br />José Arthur Benetasso Villanova<br /> 

Re: Patch: Extend NOT NULL representation to pg_constraint

From
Bernd Helmle
Date:

--On 25. September 2010 19:55:02 -0300 José Arthur Benetasso Villanova
<jose.arthur@gmail.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


Re: Patch: Extend NOT NULL representation to pg_constraint

From
Robert Haas
Date:
On Sun, Sep 26, 2010 at 2:11 PM, Bernd Helmle <mailings@oopsware.de> wrote:
> --On 25. September 2010 19:55:02 -0300 José Arthur Benetasso Villanova
> <jose.arthur@gmail.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


Re: Patch: Extend NOT NULL representation to pg_constraint

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.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


Re: Patch: Extend NOT NULL representation to pg_constraint

From
Bernd Helmle
Date:

--On 26. September 2010 15:50:06 -0400 Tom Lane <tgl@sss.pgh.pa.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


Re: Patch: Extend NOT NULL representation to pg_constraint

From
Tom Lane
Date:
Bernd Helmle <mailings@oopsware.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