Comment On Classic WTF - And I think I'll call it .... "Referential Integrity" !

Jake Vinson:

One thing I absolutely hate about working with databases is when you have to use more than one table. Don't get me wrong, I'm usually able to shove all of my data in one table, but some times you just have no choice. Thanks to Jason Strate, I'm going to be prepared next time I use multiple tables. His colleague was able to figure out a way to ensure that no one goes adding rows willy-nilly. It's sheer brilliance!

What a distinctly unique approach.  I, for one, am delighted to see a database engineer who is unconstrained by convention.  His primary concern here is the application of key business rules.  Instead of simply reading the answer out of a text book or a set of references, he was able to commit to solid principles which allowed the solution to coalesce.

You must grant me, had this problem done been solved in a normal fashion, it would have been a real clustered f*#%...

I worked with a guy who SWORE by doing all the refInt via triggers because (near quote) "The referential integrity error message are terrible! With triggers, I can put in really nice errors that explain the problem."

Apparently- the concept of making your code handle exceptions, and making it so that ref. integrity can't be violated to begin with was lost on him.

We don't need no stinkin' foreign key constraints!

In the old pre-SQL Server 6.x days when constraints were not available, real men knew how to code triggers to enforce referential integrity.  However, this ain't how it was done.  Double shame on this yahoo for using a cursor in a trigger.  What RDBMS does he think he is working with... Oracle?  Modern RDBMS's support set based processing.  Like for the last 15+ years.

Jeez.

xrT:

NOT THOSE CURSORS AGAIN...

how could anyone who know triggers and table joins do this?

oh man...

Anonymous:

xrT:

NOT THOSE CURSORS AGAIN...

how could anyone who know triggers and table joins do this?

oh man...

maybe its written for some ancient version of oracle which doesn't have RI built-in?

Just a newbie question, but couldn't the same thing be accomplished (without iterating) with a select/join/exists type test?

Jake Vinson:

Don't get me wrong, I'm usually able to shove all of my data in one table, but some times you just have no choice.

As I'm doing SQL only on rare occasions, I'm not quite sure I can follow what's going on in this trigger.  Why did they heap the Self-Referential Identity on top of the Referential Integrity?  I mean what is that

where [action].action_number = [action].root_action_number

supposed to do?  Is the real WTF here that the [action] table is not normalized, I mean, that they put almost all of their data in the [action] table but just could not seem to fit the [customer_product] stuff in there too?  Could they have turned that Self-Referential Identity into a Referential Integrity constraint between the [root_action] and [sub_action] tables?  (I'm just guessing here, I might be completely misunderstanding something...)

Well, at least he DOES checks for foreign constraints.

I'm too used to using no constraints nor foreign keys, and then coding the website that relies on that database so that it handles correctly the inconsistencies instead of pucking every time it finds something not normalized.

Like an article whose author no longer exists on the database. The "author" field has an author_id instead of a NULL, and there is no row with that id on the author table. So what? There *was* an author at some point in time. It just that his data is no longer reachable. Of course this is bad, but for small sites it works fine :) Just make sure that you won't issue new authors with old ids.

db-newbie:

Just a newbie question, but couldn't the same thing be accomplished (without iterating) with a select/join/exists type test?

I'm not exactly an expert in these matters either, but...  almost certainly, the obvious WTF would seem to be that opening a cursor, actually requesting the data, and then iterating over the returned records is pointless if you just want to know whether or not there is at least one record matching the WHERE clause.  The Sybase way to do it would probably have been

IF EXISTS (SELECT 1 FROM ... WHERE ...)

if I recall that correctly, other DBMS's might use a slightly different syntax...

Enric Naval:

Well, at least he DOES checks for foreign constraints.

I'm too used to using no constraints nor foreign keys, and then coding the website that relies on that database so that it handles correctly the inconsistencies instead of pucking every time it finds something not normalized.

Like an article whose author no longer exists on the database. The "author" field has an author_id instead of a NULL, and there is no row with that id on the author table. So what? There *was* an author at some point in time. It just that his data is no longer reachable. Of course this is bad, but for small sites it works fine :) Just make sure that you won't issue new authors with old ids.

Jeff S:

lots of WTF's:

1) use of cursor instead of NOT EXISTS() as mentioned
2) the trigger is for updated and inserted -- updated is ignored, only inserted rows are checked
3) the trigger checks to see if at least 1 row in inserted is valid. if that 1 row is valid, the trigger succeeds for all rows. It needs to check all rows, as INSERTED can be a set not just 1 row.
4) foreign key constraints can be comprised of more than 1 column. logically, if this constraint cannot be expressed as a simple FK constraint then the table structure is probably not normalized.

It is kind of hard to figure out the true business rule this person is trying acheive here, but I'm sure there's lots of WTF's all over the place in this system based on this trigger.

(lon -- he was being sarcastic saying everything should always be stored in 1 table)

Anonymous:

Alternately, this WTF could have been entitled "Who gives a FK?"

*bows in awe of your comment*

Anonymous:

Umm Yeah they never got rid of capital punishment.  I think you mean corporal punishment (after all it is rather harsh to kill somebody for bad code).

Why?

Anonymous:

Nothing like a "person customer" entity, as opposed to ... I guess ... a robot customer, or an animal customer?

I am guessing that this is to distinguish between individuals and retailers. If you have a company that sells through direct and also via retailers you would need this distinction.

Phil the ruler of heck:

db-newbie:

Just a newbie question, but couldn't the same thing be accomplished (without iterating) with a select/join/exists type test?

I'm not exactly an expert in these matters either, but...  almost certainly, the obvious WTF would seem to be that opening a cursor, actually requesting the data, and then iterating over the returned records is pointless if you just want to know whether or not there is at least one record matching the WHERE clause.  The Sybase way to do it would probably have been

IF EXISTS (SELECT 1 FROM ... WHERE ...)

if I recall that correctly, other DBMS's might use a slightly different syntax...

Septic?:

Umm Yeah they never got rid of capital punishment.  I think you mean corporal punishment (after all it is rather harsh to kill somebody for bad code).

Seriously, that made me scream out loud. That's scarier than most horror films I've seen.

Best comment yet. I want you to start writing the headlines. :)

rbanzai:

Ate my link..... :P

http://www.fluffycat.com/SQL/Cursor-Oracle/

Anonymous:

"Who gives a FK?"

Best comment yet. I want you to start writing the headlines. :)

I concur.

Anonymous:

http://www.drarok.com/Stored.txt

Seriously, that made me scream out loud. That's scarier than most horror films I've seen.

"Who gives a FK?"

Best comment yet. I want you to start writing the headlines. :)

Anonymous:

Septic?:

Umm Yeah they never got rid of capital punishment.  I think you mean corporal punishment (after all it is rather harsh to kill somebody for bad code).

maybe our friend doesn't come from your country?

we don't all execute "bad people".

i wish i knew more sql so i could figure out what's going on today... i'm guessing that a cursor iterates along a row? why is it frowned upon?

emurphy:

Anonymous:

Septic?:

Umm Yeah they never got rid of capital punishment.  I think you mean corporal punishment (after all it is rather harsh to kill somebody for bad code).

maybe our friend doesn't come from your country?

we don't all execute "bad people".

i wish i knew more sql so i could figure out what's going on today... i'm guessing that a cursor iterates along a row? why is it frowned upon?

Not only is the cursor iteration unneeded, but the entire trigger logic is unneeded.  What you're supposed to do is make the referential integrity constraint part of the table definition, something like:

constraint some_constraint foreign key (x_key) references table_x(x_key) on delete cascade

("on delete" says what to do if someone tries to delete the row in table_x being referenced; similarly "on update")

In this case, the campaign must match a "root action" i.e. a row in the 'action' table for which action_number = root_action_number, so you create a view that embodies that condition:

create view root_action as select * from action where action_number = root_action_number

and then reference the root_action view in the customer_products constraint.

seymore15074:

I concur.

"person customer" -- reminds me of that line in CSN's "Southern Cross": 

Searching for that woman girl
Who knows love can endure

As opposed to a man girl, I guess.

Enric Naval wrote the following post at 06-30-2006 1:07 PM:

...The "author" field has an author_id instead of a NULL, and there is no row with that id on the author table. So what? There *was* an author at some point in time. It just that his data is no longer reachable. Of course this is bad, but for small sites it works fine :)

Except when the invoice_header is missing for a bunch of invoice_lines, wihch would be really bad for even a "small site". Referential integrity is there to prevent logically corrupt data from entering the database. Even small sites require this. Not using RI for a DB is just as bad as putting "On Error Resume Next" on top of every ASP page you write.

Enric, you're essentially advocating two extremely dangerous programming practices. Not validating input and improperly handling or ignoring errors are the #1 and #2 causes of security vulnerabilties, and probably also the #1 and #2 causes of non-security bugs in my experience. The "size" of the site doesn't really matter much when it's 0wned or horribly broken because of bad data. Think of the issues that could arise from not using DR between a "users" table and a "user_permissions" table, even in a simple application.

Anonymous:

yeah but you wouldnt need to reference the friggen view in the first place if you just enforced a foreign key constraint

emurphy:

<...snip...>
Note that the business logic in the original WTF does not let you reference any row in this table, only one that is a root of a tree, i.e. its root_action_number points to its own action_number (so in my example, only 1 and 5 are valid targets).  Hence a view that presents only those rows.

Which is why the trigger is needed. AFAIK, you can't have foreign keys on views. So, basically, the choice here is change the data model (which may be possible, but there's not enough info to know if that's the best way here...a heirarchial table is unusual, but necessary/desirable in some cases) or enforce referential integrity through a trigger.

Now, the opening of a cursor to check for a row is inexcusable, but otherwise, I don't see a problem. Just change it to a NOT EXISTS (SELECT 1...) statement and you're good to go.

ammoQ:

A referential integrity constraint generally requires a primary key constraint on the referenced table (maybe some database systems handle that differently...). Which, in fact, implies "not null" and "unique". This trigger might be necessary because those requirements are not met. Which would in turn be another WTF, but that's another story.

Well, it implies "only 1 null" and "unique" if the PK is on a nullable column.

Anonymous:

Jeff S:

lots of WTF's:

1) use of cursor instead of NOT EXISTS() as mentioned
2) the trigger is for updated and inserted -- updated is ignored, only inserted rows are checked

3) the trigger checks to see if at least 1 row in inserted is valid. if that 1 row is valid, the trigger succeeds for all rows. It needs to check all rows, as INSERTED can be a set not just 1 row.
4) foreign key constraints can be comprised of more than 1 column. logically, if this constraint cannot be expressed as a simple FK constraint then the table structure is probably not normalized.

It is kind of hard to figure out the true business rule this person is trying acheive here, but I'm sure there's lots of WTF's all over the place in this system based on this trigger.

(lon -- he was being sarcastic saying everything should always be stored in 1 table)

this was posted, as comment when this was originally posted (see if a "classic" not posted by alex)

CAPTCHA : enterprisey

The updated table is the same as inserted. You just have a deleted table as well. Of course, he could have saved some work by checking for IF UPDATE(customer_number) OR UPDATE(campaign_number), but still that's not too terribly bad.

The set based and cursor comments are both justified though...AAMOF, I didn't catch the fact that he wasn't looping the cursor.

Anonymous:

The set based and cursor comments are both justified though...AAMOF, I didn't catch the fact that he wasn't looping the cursor.