• (nodebb)

    if not exists (frist) then insert frist; delete frist; else raise FILE_NOT_FOUND

  • Debra (unregistered)

    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...

  • Prime Mover (unregistered)

    So TRWTF is that the company Conseula works for employs people whose names are spelt incorrectly?

  • (author) in reply to Prime Mover

    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.)

  • (nodebb)

    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...

  • MiserableOldGit (unregistered)

    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.

  • MiserableOldGit (unregistered)

    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.

  • my name is missing (unregistered)

    Perhaps the code is hinting someone should delete the whole thing. Perhaps there is a trigger than replaces it with the real code.

  • (nodebb)

    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.

  • (nodebb)

    Typically I advocate for incremental improvements rather than complete rewrite. This really tests my commitment to the incremental approach.

  • Scott (unregistered)

    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.

  • Jay (unregistered)

    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@email.email', '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

  • Abigail (unregistered)

    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.

  • AnonTheMoose (unregistered) in reply to Debra

    Probably all security logging comes from triggers. Logging to a DB table, of course.

  • (nodebb)

    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).

  • (nodebb)

    Jeez, I hope to never come across something like that

  • (nodebb)

    With our code base I have no choice but to trudge along similarly...

  • (nodebb)

    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.

  • (nodebb) in reply to Loren Pechtel

    This is a workaround using cascading deletes.

    What, exactly, is it that you think this procedure deletes?

  • James (unregistered)

    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!

  • MiserableOldGit (unregistered) in reply to Jaime
    What, exactly, is it that you think this procedure deletes?

    Sanity, by the bucket load.

  • Allan Mills (google)

    I'm a little confused. The article says it is from a .NET project. But that looks like PL/SQL code to me.

  • ZZartin (unregistered)

    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.

  • Wizofaus (unregistered)

    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.

  • (nodebb) in reply to Wizofaus

    and any existing record with that ssoid

    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.

  • (nodebb) in reply to Abigail

    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.

    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.

  • (nodebb) in reply to Allan Mills

    Better look again. This is definitely not pl/sql. T-sql is more likely, but I am not an expert on that.

  • (nodebb) in reply to Loren Pechtel

    For some reason they do not have permission to delete users.

    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.

  • Allan Mills (google) in reply to nerd4sale

    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.

  • (nodebb) in reply to Allan Mills

    Not PL/SQL. One of my biggest gripes about Microsoft's T-SQL is that they haven't stolen this syntax from Oracle:

    declare
      v_username Users.Username%TYPE
    

Leave a comment on “Delete This”

Log In or post as a guest

Replying to comment #:

« Return to Article