Re: [PATCHES] Foreign key type checking patch - Mailing list pgsql-hackers

Hello Stephan,

> > CREATE TABLE foo(fid INT4 NOT NULL PRIMARY KEY, ...);
> > CREATE TABLE bla(fid INT2 REFERENCES foo, ...);
> >
> > The application will be fine till you enter fid=32767, and
> > it inserts will fail in bla with fid=32768. Much later on.
>
> Which is fine if bla is meant to store a subset of the allowable foo
> values.
> [...]

Sure. This is NOT my point. I totally agree with you that the above
example MAY BE what the user intends, and that it must be allowed.
However it may ALSO be a bug that will pop up later on.

Although it is POSSIBLE that this is fine, it is much more PROBABLE that
it is a bug, hence I just suggest to issue a mere simple basic plain
user-friendly little warning, what is quite different from issuing an
error.

Thus, the user has the information. He may chose to go on as that is what
was meant, or maybe check the stuff and correct it.

In postgres compilation, gcc uses the -Wall option to issue warnings about
correct C constructs that may hide application bugs. This is the
philosophy I'm suggesting here for this very small feature.
"Dear user, what you ask is right, however it looks a little bit strange,
so I tell you just in case." I'm sure you're pretty happy that the gcc
developers put such features for basic programmers, and that you use
them;-) Why not allowing that kind of approach in postgres?

> > CREATE TABLE foo(fid VARCHAR(4) NOT NULL PRIMARY KEY, ...);
> > CREATE TABLE bla(fid VARCHAR(8) REFERENCES foo, ...);
> >
> > declaring a larger size is not a problem here, however you will
> > never be able to but any reference in bla larger than 4 as it must
> > match its counter part in foo. So it is just a little bit stupid.
>
> This one is fairly pointless

Isn't it what I'm saying?

> for the single column case but a multiple column match unspecified
> constraint could allow the full 8 characters if there's a second column
> which is null.

I do not understand. I can't see how you can put 8 characters in a
reference which must match a 4 characters string.

> > CREATE TABLE day(quand DATE NOT NULL PRIMARY KEY, ...);
> > CREATE TABLE event(quand TIMESTAMP REFERENCES day, ...);
> >
> > The intent could be that events should refer to some day already
> > registered in the base. Obviously it does work, because the = will cast to
> > timestamp, to only the 00:00:00 timestamp will match a day.
>
> This one does seem fairly broken.

Yes, my comment is broken, as I wrote "it does work", meaning, "it does
not work":-(

Anyway postgres accepts the two tables and the foreign key references from
a timestamp to a date without a warning, and although I can imagine an
application that would really mean that, I'm pretty sure most users would
like this reported as a warning, as it is more likely to be a bug.

> CREATE DOMAIN posint AS int4 check(value>0);
> CREATE TABLE foo(fid int4 primary key);
> CREATE TABLE bla(fid posint references foo);
>
> The intent here is that foo may contain negative numbers but that those
> rows won't be referenced by bla. This is similar to 1 and 2.

Sure, I agree with you there are example where you may mean that.
My point is about a ***warning***, because I think that most of the time
this hides a future bug, even if some time it may be the intent.

Have a nice day,

-- 
Fabien Coelho - coelho@cri.ensmp.fr