SEO

Hi Alex,

On Sun, Mar 30, 2008 at 7:10 AM, Alex Hunsaker <badalex@gmail.com> wrote:

> (trimmed cc's)
>
> Find attached inherited_constraint_v2.patch
>
> Changes since v1:
> -rebased against latest HEAD
> -changed enum { Anum_pg_constraint_... } back into #define
> Anum_pg_constraint_...
> -remove whitespace damage I added
> -fixed regression tests I added to be more robust
> -fixed
> create table ac (a int constraint check_a check (a <> 0));
> create table bc (a int constraint check_a check (a <> 0)) inherits (ac);
> so it properly works (removed crud I put into
> AddRelationRawConstraints and created a proper fix in DefineRelation)
>

I was taking a look at this patch to add the pg_dump related changes. Just
wanted to give you a heads up as this patch crashes if we run "make
installcheck". Seems there is an issue introduced in the CREATE TABLE
REFERENCES code path due to your patch (this is without my pg_dump changes
just to be sure). Looks like some memory overwrite issue. The trace is as
follows:

Core was generated by `postgres: nikhils regression [local] CREATE
TABLE '.
Program terminated with signal 11, Segmentation fault.
#0 0x08378024 in AllocSetCheck (context=0xa060368) at aset.c:1112
1112 if (dsize > 0 && dsize < chsize &&
*chdata_end != 0x7E)
(gdb) bt
#0 0x08378024 in AllocSetCheck (context=0xa060368) at aset.c:1112
#1 0x0837704f in AllocSetDelete (context=0xa060368) at aset.c:487
#2 0x083783c2 in MemoryContextDelete (context=0xa060368) at mcxt.c:196
#3 0x083797fb in PortalDrop (portal=0xa0845bc, isTopCommit=0 '\0') at
portalmem.c:448
#4 0x08281939 in exec_simple_query (
query_string=0xa07e564 "CREATE TABLE enumtest_child (parent rainbow
REFERENCES enumtest_parent);") at postgres.c:992
#5 0x082857d4 in PostgresMain (argc=4, argv=0x9ffbe28, username=0x9ffbcc4
"nikhils") at postgres.c:3550
#6 0x0824917b in BackendRun (port=0xa003180) at postmaster.c:3204
#7 0x082486a2 in BackendStartup (port=0xa003180) at postmaster.c:2827
#8 0x08245e9c in ServerLoop () at postmaster.c:1271
#9 0x082457fd in PostmasterMain (argc=3, argv=0x9ff9c60) at postmaster.c
:1019
#10 0x081e1c03 in main (argc=3, argv=0x9ff9c60) at main.c:188


Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

On Mon, Mar 31, 2008 at 2:36 AM, NikhilS <nikkhils@gmail.com> wrote:
> Hi Alex,

> I was taking a look at this patch to add the pg_dump related changes. Just
> wanted to give you a heads up as this patch crashes if we run "make
> installcheck". Seems there is an issue introduced in the CREATE TABLE
> REFERENCES code path due to your patch (this is without my pg_dump changes
> just to be sure). Looks like some memory overwrite issue. The trace is as
> follows:

Ouch, sorry i did not reply sooner... been out with the flu. Oddly
enough make check and make installcheck worked great on my 64 bit box.
But on my laptop(32 bits) make check lights up like a christmas tree.
Which is why I did not notice the problem.

Attached is a patch that fixes the problem... (it was debugging from
an earlier version)


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

"Alex Hunsaker" <badalex@gmail.com> writes:
> [ patch to fix behavior of inherited constraints ]

Looking over this patch, I see that it introduces a syscache on
pg_constraint (conrelid, conname), which requires a unique index
underlying it. This is not workable because domain constraint
entries in pg_constraint will have conrelid = 0. The index would
therefore have the effect of forbidding the same constraint name
to be used for two different domains' constraints.

The fact that pg_constraint stores both relation and domain constraints
is a fairly ugly crock, not least because it means there is no natural
primary key for the table. I've thought for some time that we should
split it into two catalogs. (We could provide a union view to avoid
breaking clients that look at it.) However it seems a bit ill-advised
to tackle that change as an essential part of this patch.

Was there any particularly strong reason why you introduced the syscache
instead of working with the available indexes?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, May 6, 2008 at 7:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Alex Hunsaker" <badalex@gmail.com> writes:
> > [ patch to fix behavior of inherited constraints ]
>
> Looking over this patch, I see that it introduces a syscache on
> pg_constraint (conrelid, conname), which requires a unique index
> underlying it. This is not workable because domain constraint
> entries in pg_constraint will have conrelid = 0. The index would
> therefore have the effect of forbidding the same constraint name
> to be used for two different domains' constraints.
>
> The fact that pg_constraint stores both relation and domain constraints
> is a fairly ugly crock, not least because it means there is no natural
> primary key for the table. I've thought for some time that we should
> split it into two catalogs. (We could provide a union view to avoid
> breaking clients that look at it.) However it seems a bit ill-advised
> to tackle that change as an essential part of this patch.
>
> Was there any particularly strong reason why you introduced the syscache
> instead of working with the available indexes?
>
> regards, tom lane

None other than the syscache stuff was way easier to work with than
the 25-50 lines of boilerplate code that Ill need everywhere I use
CONSTRNAME. (see the hunk to MergeAttributesIntoExistsing for an
example of what i mean). Not a big deal though, NikhilS was not sure
about those changes in the first place.

Ill just rip it out for now. Patch forthcoming.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers