SQL procedure re-write

  • First I am not an expert on sql programming, but know enough to get the job done. I have inherited an application with one procedure that has bugged me and I have been thinking of rewriting it for some time. But I'm clueless as to how to improve it.

    We have a table of users that contains millions of records. When a user is authenticated we run procedure that looks like the the following code

    [p]--if email doesn't exist, return -1

    if not exists (select * from users where id=@ID and email=@Email) begin

    select userid = -1

    end

    -- if wrong password, return -2

    else if (select Password from users where id=@ID and email=@Email) != @pass begin

    select userid = -2

    end

    -- if password matches, return userid

    else begin

    select * from users where id = @ID and email = @email

    end[/p]

    I think this is not the most efficient code, because for a successful id,pass,email combination, we are running three queries. First to check if it exist, then to check if password matches, then to retrieve the recordset ( the application gets the full recordset).

    This is how I would rewrite it

    1. run a single query with the id and admin to retrieve the record

    2. If the record returned check for pass, if matches return the recordset, else return -2

    3. else return -1

    How do I write such procedure?

  • I think you're on the right track. How about something like (data types would probably need to be adjusted & this is psuedo-code)...

    DECLARE @ComparePwd nvarchar(10), @CompareEmail nvarchar(100)

    SELECT @ComparePwd = u.Pwd

    ,u.*

    FROM users u

    WHERE u.Id = @Id

    AND u.Email = @Email

    IF @@ROWCOUNT = 0

    RAISERROR ("Bad email address")

    IF @ComparePwd <> @Pwd

    RAISERROR("Bad password")

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • Or - set the return status of the SP so that the calling app knows why it didn't work....

    (not a huge fan of raiserror)

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • Grant,

    Thanks for that code. I can understand what you trying to do.

    when I try to do this though, I get the following error

    A SELECT statement that assigns a value to a variable must not be combined with data-retrieval operations.

    Is this mean I can not run a single query to determine all three cases. Do I have to run two queries?

    When I run a select query, is there a way to read the returned record? If I can do that, then I can compare the given pwd with that of the record.

  • You can dump the one record into a table var, and read that.

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • Matt Miller (2/28/2008)


    You can dump the one record into a table var, and read that.

    I know I can dump the one record into a table. To do that I need to match the User Table schema with the temp table. If the User Table has many columns, which it does, then is there a way to create a temp table from table schema? What happens if the user table schema changes?

  • Sorry, I was spending more time thinking about the logic than the syntax. You can cut my pay for this job. 😉

    How about something like this (this assumes one row will be returned from the ID & Email pair and this time I checked the syntax):

    SELECT u.* INTO #CheckTable

    FROM dbo.User u

    WHERE u.Id = @Id

    AND u.Email = @Email

    IF @@ROWCOUNT = 0

    RETURN -5 -- Bad Email

    SELECT @CheckPwd = c.Pwd

    FROM #CheckTable c

    IF @CheckPwd <> @Pwd

    RETURN -6 -- Bad Password

    SELECT * FROM #CheckTable

    DROP #CheckTable

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • yusuf (2/28/2008)


    Matt Miller (2/28/2008)


    You can dump the one record into a table var, and read that.

    I know I can dump the one record into a table. To do that I need to match the User Table schema with the temp table. If the User Table has many columns, which it does, then is there a way to create a temp table from table schema? What happens if the user table schema changes?

    Grant beat me to it - so you can ding me for not being clear enough...:).

    The Select...INTO creates a new table based on whatever columns you include. Using Grant's syntax just above - you don't "have" to worry about schema changes - the temp table will have whatever the schema has at that time.

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • Grant , Matt

    Thank you for your help. The procedure works like a charm.

    I run before and after client statistics and boy did that make huge difference. I knew the procedure was not right and I have even heard some people complain about the responsiveness of the application. Wait until they see the change.

    Again thanks for your help. 🙂

  • Not a problem.

    You want one more, take a look at the execution plan and be sure that it's using a good index to select the record. You might be doing a table scan today and eliminating that will give you an even bigger increase. You'll be a hero.... for about 10 or 15 minutes until some other process hangs or a user doesn't have permission to a stored procedure or a nightly load fails and then it's back to the dog house.

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

Viewing 10 posts - 1 through 10 (of 10 total)

You must be logged in to reply to this topic. Login to reply