Want to win a PS4? Go Premium and enter to win our High-Tech Treats giveaway. Enter to Win

x
?
Solved

Check SQL Code

Posted on 2008-10-14
9
Medium Priority
?
195 Views
Last Modified: 2012-05-05
Hi
I have the following code, just really want to know whether it the most efficient way.

ALTER PROCEDURE [dbo].[Activate_Account]

      @MemberID VarChar(50) = '4b95187c-00ee-453b-9daf-456c3952da77'
AS

DECLARE @CurrentTime DateTime
SET @CurrentTime = GetDate()
DECLARE @MemberAccountCreated DateTime
DECLARE @TimeDiff INT
DECLARE @ReturnCode INT
SET @ReturnCode = 0

BEGIN
      
      SET NOCOUNT ON;

SELECT @MemberAccountCreated = dbo.Membership.MemberAccountCreated
         FROM dbo.Membership
WHERE MemberUserID = @MemberID

SELECT @TimeDiff = CAST(a - b AS numeric(18,2))
            FROM (
                  SELECT
                        CAST(@CurrentTime AS datetime) AS a,
                        CAST(@MemberAccountCreated AS datetime) AS b
                  )AS TimeDiff

UPDATE dbo.Membership
      SET MemberActivateAccount = 1
WHERE MemberUserID = @MemberID AND @TimeDiff < 2

IF(@TimeDiff > 2)
      BEGIN
SET @ReturnCode = 1
      END
ELSE IF(@@RowCount > 0)
      BEGIN
SET @ReturnCode = 2
      END
ELSE
      BEGIN
SET @ReturnCode = 3
      END

SELECT @ReturnCode AS ReturnCode
END
0
Comment
Question by:BeginningWebDesign
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 4
  • 4
9 Comments
 
LVL 39

Expert Comment

by:BrandonGalderisi
ID: 22717057
Let's go through this step by step
ALTER PROCEDURE [dbo].[Activate_Account] 
      @MemberID VarChar(50) = '4b95187c-00ee-453b-9daf-456c3952da77'
AS
 
DECLARE @CurrentTime DateTime
SET @CurrentTime = GetDate()
DECLARE @MemberAccountCreated DateTime
DECLARE @TimeDiff INT
DECLARE @ReturnCode INT
SET @ReturnCode = 0
 
BEGIN
      
      SET NOCOUNT ON;
 
--just for my sanity, why qualify a single column single table select :)
SELECT @MemberAccountCreated = MemberAccountCreated
FROM dbo.Membership
WHERE MemberUserID = @MemberID
 
--this is an elaborate datediff.
SELECT @TimeDiff = CAST(CurrentTime  - MemberAccountCreated  AS numeric(18,2))
            FROM (
                  SELECT
                        CAST(@CurrentTime AS datetime) AS a,
                        CAST(@MemberAccountCreated AS datetime) AS b
                  )AS TimeDiff
 
/*
--this is a simpler datediff
--and testing here instead of in the where clause will prevent an index seek on memberuserid in the table since you will only actually do the update when the @timediff was <2
*/
if datediff(d, @CurrentTime,@MemberAccountCreated)<2
begin
UPDATE dbo.Membership
set MemberActivateAccount = 1
WHERE MemberUserID = @MemberID
if @@rowcount>0
set @returnCode=2
else
set @returnCode=3
end
else 
set @returncode=1
--what about if @timediff=2.  You are testing for >2 and <2 but =2 will fall into the else, which is 3
/*
IF(@TimeDiff > 2)
      BEGIN
SET @ReturnCode = 1
      END
ELSE IF(@@RowCount > 0)
      BEGIN 
SET @ReturnCode = 2
      END
ELSE
      BEGIN
SET @ReturnCode = 3
      END
*/
SELECT @ReturnCode AS ReturnCode
END

Open in new window

0
 

Author Comment

by:BeginningWebDesign
ID: 22718588
Hi BrandonGalderisi:
Thanks, the code returns 2(success) even when the date is greater than 2, it should return 1
George
0
 
LVL 51

Expert Comment

by:Mark Wills
ID: 22719653
OK, we know that memberaccountcreated is meant to be a datetime...

because of : SELECT @MemberAccountCreated = dbo.Membership.MemberAccountCreated  FROM dbo.Membership...

So, we also know that CurrentTime is similarly a legitimate datetime, so not sure whay the "CAST" is required, so we can achieve a lot in one statement...


UPDATE Membership SET MemberActivateAccount = 1
WHERE MemberUserID = @MemberID 
AND datediff(dd,MemberAccountCreated, getdate()) < 2 
 
IF(@@RowCount > 0)
   SET @ReturnCode = 2
ELSE
   SET @ReturnCode = 1
 
-- It would appear that returncode = 1 is an un-updateable query where data exists, just not within 2 days. 
-- Similarly, returncode of 3 means similar things, but could also mean that member does not exist. Do you really need that three part flag ?

Open in new window

0
Visualize your virtual and backup environments

Create well-organized and polished visualizations of your virtual and backup environments when planning VMware vSphere, Microsoft Hyper-V or Veeam deployments. It helps you to gain better visibility and valuable business insights.

 

Author Comment

by:BeginningWebDesign
ID: 22725136
Hi mark_wills:

ReturnCode 1 = Successfull update, MemberUserID exists and account is activated within 2 days.
ReturnCode 2 = MemberUserID exists, but new member has not activated account within the 2 day limit so no update has taken place.
ReturnCode 3 = No update has taken place not really bothered why, just inform new member that cannot activate account

Hope this helps
George
0
 

Author Comment

by:BeginningWebDesign
ID: 22726379
Sorry my mistake wrong way round, should read

ReturnCode 1 = MemberUserID exists, but new member has not activated account within the 2 day limit so no update has taken place.

ReturnCode 2 = Successfull update, MemberUserID exists and account is activated within 2 days.

ReturnCode 3 = No update has taken place not really bothered why, just inform new member that cannot activate account

George
0
 
LVL 51

Expert Comment

by:Mark Wills
ID: 22728136
So, 1 and 3 are essentially the same, except that 3 no longer has an opportunity to activate and really, the only time that is different to 1 is where memberid does not exist - so really - what is the difference ?
0
 

Author Comment

by:BeginningWebDesign
ID: 22728391
Hi Mark
This is what i should have wrote, 3 will only be returned if an error occured, 2 if update success and 1 if update failed due to date diff been more than 2 days

IF(@@RowCount > 0)
   SET @ReturnCode = 2
ELSE
   SET @ReturnCode = 1

SELECT @Error = @@Error

IF @Error != 0
      BEGIN
            SET @ReturnCode = 3
      END
 SELECT @ReturnCode AS ReturnCode
END

Questions is, will @Error ever run as i have tried a few examples and all have failed getting @ReturnCode to = 3

George
0
 
LVL 51

Expert Comment

by:Mark Wills
ID: 22730124
I think a return code of 3 will only ever be legitimate if @MemberID does not exist (looking at the very original) or, the @MemberID is held under lock and the update has to time out / fail...

So, it is possible for a legitimate error to happen... but outcome / followup action is more like returncode =1  ( meaning please update your membership before it is too late )...

Now if @memberid does not exist, or more than 'nn' days. Could check for 'cleanup' actions... maybe a returncode of 4 ?

Your call, and can very simply add error checking, the reason why you are not getting errors is possibly because how it is being checked. Need to capture both at the same time (in yours the @@error is responding to the IF statement).
DECLARE @ErrorVar INT, 
        @RowCountVar INT;
 
UPDATE Membership SET MemberActivateAccount = 1
WHERE MemberUserID = @MemberID 
AND datediff(dd,MemberAccountCreated, getdate()) < 2 
 
SELECT @ErrorVar = @@ERROR, @RowCountVar = @@ROWCOUNT;
 
IF(@RowCountVar > 0)
   SET @ReturnCode = 2
ELSE
IF (@errorVar > 0
   SET @ReturnCode = 3
ELSE
   SET @ReturnCode = 1
 

Open in new window

0
 
LVL 51

Accepted Solution

by:
Mark Wills earned 2000 total points
ID: 22730127
forgot the closing bracket in lucky line 13
0

Featured Post

Get free NFR key for Veeam Availability Suite 9.5

Veeam is happy to provide a free NFR license (1 year, 2 sockets) to all certified IT Pros. The license allows for the non-production use of Veeam Availability Suite v9.5 in your home lab, without any feature limitations. It works for both VMware and Hyper-V environments

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

In SQL Server, when rows are selected from a table, does it retrieve data in the order in which it is inserted?  Many believe this is the case. Let us try to examine for ourselves with an example. To get started, use the following script, wh…
If you have heard of RFC822 date formats, they can be quite a challenge in SQL Server. RFC822 is an Internet standard format for email message headers, including all dates within those headers. The RFC822 protocols are available in detail at:   ht…
Have you created a query with information for a calendar? ... and then, abra-cadabra, the calendar is done?! I am going to show you how to make that happen. Visualize your data!  ... really see it To use the code to create a calendar from a q…
Please read the paragraph below before following the instructions in the video — there are important caveats in the paragraph that I did not mention in the video. If your PaperPort 12 or PaperPort 14 is failing to start, or crashing, or hanging, …

609 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question