427936 - update sanitycheck.cgi to include missing foreign keys cross checks

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Noura Elhawary]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.9) Gecko/20070209 Fedora/1.5.0.9-3.fc6 Firefox/1.5.0.9 pango-text
Build Identifier: 

the foreign keys cross checks in sanitycheck.cgi are missing some checks for newly added FKs to the Bugzilla/DB/Schema.pm , we need to include those checks.

Reproducible: Always

Comment 1

[Noura Elhawary]

Attachment: v1 patch to add missing FK cross checks to sanitycheck.cgi

this bug depends on other bugs that update Bugzilla/DB/Schema.pm with the missing FKs

Comment 2

[Max Kanat-Alexander]

No, the FKs don't need to be enforced by sanitycheck.cgi once they're in the Schema. That's the whole point of having real FKs. In fact, anything that has an FK in the Schema should be *removed* from sanitycheck.cgi.

Comment 3

[Noura Elhawary]

(In reply to comment #2)
> No, the FKs don't need to be enforced by sanitycheck.cgi once they're in the
> Schema. That's the whole point of having real FKs. In fact, anything that has
> an FK in the Schema should be *removed* from sanitycheck.cgi.
> 

I understand that the FKs in Bugzilla/DB/Schema.pm will enforce the fks on newly inserted data into the database but how about if the database have already bad values that were there before creating the FKs in Bugzilla/DB/Schema.pm ? shouldn't sanitycheck.cgi show those values?

Comment 4

[Max Kanat-Alexander]

(In reply to comment #3)
> I understand that the FKs in Bugzilla/DB/Schema.pm will enforce the fks on
> newly inserted data into the database but how about if the database have
> already bad values that were there before creating the FKs in
> Bugzilla/DB/Schema.pm ? shouldn't sanitycheck.cgi show those values?

  The database (and checksetup) forbids the creation of the FK if there are invalid values in the relationship.

Comment 5

[Max Kanat-Alexander]

Okay, talking about it with Noura, I think these would be good for 3.2, so that people can know there's something wrong with their DB *before* they go into checksetup and do an upgrade.

Comment 6

[Max Kanat-Alexander]

That also means that none of the blockers are actually blockers.

Comment 7

[Noura Elhawary]

Attachment: v2 patch to add missing FK cross checks to sanitycheck.cgi

removed the depends on comments. a

Comment 8

[Max Kanat-Alexander]

Comment on attachment 314521 [details] [diff] [review]
v2 patch to add missing FK cross checks to sanitycheck.cgi

LpSolit knows more about sanitycheck than me.

Comment 9

[Frédéric Buclin]

Comment on attachment 314521 [details] [diff] [review]
v2 patch to add missing FK cross checks to sanitycheck.cgi

>+           ["flagtypes", "grant_group_id"],
>+           ["flagtypes", "request_group_id"]);

Nit: we may pass "id" as 3rd argument so that the flagtype ID is also displayed, but many other places do not do it. So r=LpSolit with or without this additional 3rd argument.

Comment 10

[Frédéric Buclin]

CrossCheck() will go away in 4.0 when all FK are in place. But for now, they all stay.

Comment 11

[Frédéric Buclin]

Checking in sanitycheck.cgi;
/cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v  <--  sanitycheck.cgi
new revision: 1.140; previous revision: 1.139
done