- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
if not exists (frist) then insert frist; delete frist; else raise FILE_NOT_FOUND
Admin
I can bet my sorry tushy that there are some crucial triggers which get called on these inserts and deletes, which is why doing it this way is needed.
Whether it's a good thing, well... erm...
Admin
So TRWTF is that the company Conseula works for employs people whose names are spelt incorrectly?
Admin
Funny thing on this one: sometimes, when the submitter wishes to be anonymous, I use a random name generator to give them a pseudonym. That was how the name generator popped it out, but yeah, I should have caught that. (The first time I put the name in, I paste it, and every other time, VS Code autocompletes it, and yes, I write articles in VS Code.)
Admin
An apparent WTF.... but (and this is a stretch) it could be part of testing code for what happens when a user is deleted. The "stuff" is to make sure they are dealing with a "temporary" rather than "real" user...
Admin
I suspect it's one of those user tables where nothing is ever really deleted, probably got some horrible triggers auditing the hell out of everything and turning a delete from instruction into an update flag so user rows are always kept, but hidden. A mess, but not an uncommon one, this makes this procedure one of those WTFs that happens when someone can't get their head around the existing functionality and tries to correct a bug or implement a required change with a truly awful hack. That would mean this procedure really should be named usermgt_append_deleted_user, although as @username will be empty in that insert it doesn't even do that. I think need a little lie down in a darkened room.
Admin
Looking at it again, username might or might not be empty, but if it isn't empty, there's some other mess in this system.
Admin
Perhaps the code is hinting someone should delete the whole thing. Perhaps there is a trigger than replaces it with the real code.
Admin
Another bit of "interestingness"; the error has a severity of 10. This means that the caller won't see it as an actual error, but a warning - and warnings are almost always swept under the rug.
So, for the average caller, this procedure simply doesn't do anything.
Admin
Typically I advocate for incremental improvements rather than complete rewrite. This really tests my commitment to the incremental approach.
Admin
I'm really appreciating some of the fine details here.
They declare @username, and select if from the Users table. Why? So that they can Insert it into SSOData (and promptly delete it). On the other hand, hard-coded dummy values are fine for email and name. Why? Maybe somebody knows. Maybe not.
And is the Users table in sync with the SSOData table? What happens if @username comes up Null? So many fascinating mysteries here.
Admin
I think i see what they were trying to do. This method is supposed to align the ssodata table with the users table, except that it doesn't do anything because it's logic is wrong. The check doesn't check if the username is paired with the ssoid and the DELETE just removes all records with that ssoid, so it does absolutely nothing if the ssoid doesn't exist except add noise to any logs attached and if it DOES exist it just throws an error.
I'd rewrite it this way:
CREATE proc [dbo].[usermgt_DeleteUser] ( @ssoid uniqueidentifier ) AS begin declare @username nvarchar(64) select @username = Username from Users where SSOID = @ssoid if (not exists(select * from ssodata where ssoid = @ssoid and UserName = @username)) begin insert into ssodata (SSOID, UserName, email, givenName, sn) values (@ssoid, @username, '[email protected]', 'Firstname', 'Lastname') delete from ssodata where ssoid = @ssoid and not (UserName = @username) end else begin RAISERROR ('This user still exists in sso', 10, 1) end
Admin
My bet is that there is a foreign key constraint between the tables Users and ssodata, and this is procedure to remove rows from Users which don't have an associated row in ssodata. The procedure should not be called for rows which are present in ssodata -- those rows are ok. If the foreign key relation between Users and ssodata is set up with cascading deletes, then creating an entry in ssodata and then deleting it, removes the corresponding rows from Users. And it will create the relevant audit records (be it by triggers, replication, or whatever they have in place) by deleting from ssodata instead of directly from Users. Or it may bypass anything which is triggered from directly deleting from Users.
Admin
Probably all security logging comes from triggers. Logging to a DB table, of course.
Admin
Umm … could you please spell Consuela's name correctly?
Anyway, this code is rather benign. It doesn't do anything, thus no data gets destroyed (always a plus on this website) other than detect whether the user exists, which probably is used somewhere to really delete the user (after catching the error – probably in the error handler itself).
Admin
Jeez, I hope to never come across something like that
Admin
With our code base I have no choice but to trudge along similarly...
Admin
I think I see a shred of sanity here:
For some reason they do not have permission to delete users. This is a workaround using cascading deletes.
Admin
What, exactly, is it that you think this procedure deletes?
Admin
In SQL Server there can be only one schema. Many think dbo stands for database owner; this is naive, as it clearly stands for database one (schema). Pay no attention to that sys behind the curtain!
Admin
Sanity, by the bucket load.
Admin
I'm a little confused. The article says it is from a .NET project. But that looks like PL/SQL code to me.
Admin
Yeah it's a decent bet there's something that triggers on deletes from the sso table. It's taking in a guid so it had to some from somewhere, probably that's how they're preventing the triggers from running if there's actually already a record in there which makes it a very badly named function. And the RAISEERROR is just a glorified PRINT statement because of the severity it's running with.
Admin
If ssoid isn't a unique key in ssodata then it could do something useful - the insert is pointless (triggers aside) but the delete may well delete both the record just inserted and any existing record with that ssoid. But only if the user had first been deleted from the Users table of course.
Admin
That's one of things that makes me appreciate the layers of WTF on this one... Our brain really wants to make sense of this, so we reach that conclusion... Except we're in a
if (not exists(select * from ssodata where ssoid = @ssoid))block, so there aren't any other rows with that ssoid.Admin
If there is a foreign key constraint between Users and ssodata with ssodata as the primary table, then there can't be any rows in Users which don't have a corresponding row in ssodata.
If there is a foreign key constraint between Users and ssodata with Users as the primary table, there may be rows in Users which don't have a corresponding row in ssodata, but deletes won't cascade from ssodata to Users...
Something weirder is going on here.
Admin
Better look again. This is definitely not pl/sql. T-sql is more likely, but I am not an expert on that.
Admin
Another funny MSSQL fact: if the trigger could do the delete, then the procedure could do the delete. Both triggers and stored procedures follow the same rules on ownership chaining and permissions. So, even if the user calling the procedure didn't have permission to delete rows, a workaround is still not necessary.
Admin
Might not be PL/SQL, it was the first programming language I worked with when I entered the workforce but that was decades ago. It does look similar though.
Admin
Not PL/SQL. One of my biggest gripes about Microsoft's T-SQL is that they haven't stolen this syntax from Oracle:
Admin
Did the post get fixed, so now this comment looks bizarre? Or did you know someone named Conseula, despite the fact that most people named similarly are Consuela?
In any event, I've met quite a few people in real life who "spell their name wrong" according to some other people. I've generally sided with those people who asserts that it's the individual with the name that gets to determine what the correct spelling and pronunciation of their name is. The correct pronunciation they decide on does not have to be one that they can actually utter. Most of the examples I've encountered of that sort of thing aren't linkable, since most personally experienced verbal encounters aren't online. But there's also the Leif and Thorn summer-sunshine-4084 update.