asked on

making stored procedure more efficient

Hi there

Here i have stored procedure that works fine. My question is about making it more efficent and using transactions as it is insert records etc, i know there is also an error thing i can use in case one of them fails. Can anyone give me some advice or point me in teh right direction. Amendment of my code to reflect these changes would be much appreciated

thanks in advance

davoman

------------------------------------------------
CREATE PROCEDURE CheckQualifying @ProductCode varchar(10),
@cust_code varchar(10), @amount decimal(10,2)
AS
Declare @count int
DECLARE @PromotionID int
DECLARE @VAlues int

SET @count =(
SELECT Count(prm_ql_pr_code)
FROM dbo.is_prm_hd INNER JOIN dbo.is_prm_ql
ON prm_ql_hd_id=prm_hd_id
WHERE (getdate() between prm_hd_start AND prm_hd_end)
AND prm_ql_pr_code = @ProductCode)

If @count > 0
      BEGIN

      SET @PromotionID =(SELECT prm_hd_id
FROM dbo.is_prm_hd INNER JOIN dbo.is_prm_ql ON prm_ql_hd_id=prm_hd_id
WHERE (getdate() between prm_hd_start AND prm_hd_end) AND prm_ql_pr_code = @ProductCode)

      SET @values = (SELECT prm_ql_pv
FROM dbo.is_prm_hd INNER JOIN dbo.is_prm_ql ON prm_ql_hd_id=prm_hd_id
WHERE (getdate() between prm_hd_start AND prm_hd_end) AND prm_ql_pr_code = @ProductCode)

      INSERT INTO is_prm_temp_values (prm_temp_acc_code,prm_temp_prm_id,prm_temp_value)
            VALUES (@cust_code,@PromotionID,@values*@amount)

      END

IF (SELECT sum(prm_temp_value) FROM dbo.is_prm_temp_values
      WHERE prm_temp_acc_code=@cust_code AND prm_temp_prm_id = @PromotionID)
      >= (SELECT prm_qual_tot FROM dbo.is_prm_hd WHERE prm_hd_id=@PromotionID)
BEGIN
UPDATE dbo.is_cust_prm_hd
SET cust_prm_qualified = -1
WHERE cust_prm_hd_id =@PromotionID
END
------------------------------------------

adathelad

Hi,

You can cut that down a bit to reduce the number of selects/checks you are doing. Try this:

CREATE PROCEDURE CheckQualifying @ProductCode varchar(10),
@cust_code varchar(10), @amount decimal(10,2)
AS
Declare @count int
DECLARE @PromotionID int
DECLARE @VAlues int

SELECT @PromotionID = prm_hd_id,
      @values = prm_ql_pv
FROM dbo.is_prm_hd
      INNER JOIN dbo.is_prm_ql ON prm_ql_hd_id=prm_hd_id
WHERE prm_hd_start <= GETDATE()      AND prm_gd_end >= GETDATE() AND prm_ql_pr_code = @ProductCode

IF (@@ROWCOUNT > 0)
      BEGIN
             INSERT INTO is_prm_temp_values (prm_temp_acc_code,prm_temp_prm_id,prm_temp_value)
     VALUES (@cust_code,@PromotionID,@values*@amount)
     END

IF (SELECT sum(prm_temp_value) FROM dbo.is_prm_temp_values
WHERE prm_temp_acc_code=@cust_code AND prm_temp_prm_id = @PromotionID)
>= (SELECT prm_qual_tot FROM dbo.is_prm_hd WHERE prm_hd_id=@PromotionID)
BEGIN
UPDATE dbo.is_cust_prm_hd
SET cust_prm_qualified = -1
WHERE cust_prm_hd_id =@PromotionID
END

ASKER CERTIFIED SOLUTION

adathelad

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

davoman

ASKER

Would that be the correct to do this

I mean should I have the transactions in there for this sort of procedure, i'm often aware I should have them but never quite sure why

thanks for your help

adathelad

Transactions should be used when you want to ensure that all or nothing succeeds. It's a good idea to get in the habit of using transactions like this - if the procedure contained just one INSERT statement, then you could argue that you don't need to use transactions as there are no other data modification tasks being done in the same batch.

If you don't use transactions when performing multiple data modification tasks in one batch, then you run the risk of having some, but not all, of the actions completing successfully. This could lead to data inconsistency/corruption

davoman

ASKER

That sounds great to me and the code works great thanks

one more question if i may induldge

ROLLBACK TRANSACTION
RETURN

what does this return actually do.

can i somehow send a result back to the program so the user knows the procedure has failed. Thanks again

adathelad

>> what does this return actually do. <<
It causes the procedure to exit (i.e. doesn't execute anything after that point)

>> can i somehow send a result back to the program so the user knows the procedure has failed. <<
You can either,
1) Use RETURN to return an INTEGER indicating status

i.e.
RETURN 1 if there's an error (i.e. in the "IF @@ERROR <> 0" blocks)
RETURN 0 at the very end of the procedure after the COMMIT TRANSACTION (to indicate success)

You can then capture this return value (i.e. EXECUTE @Result = MyProcedureName)

Or....
2) Add an OUTPUT parameter to the stored procedure and set this variable to an error code/message if an error occurs. Then check the value of this in the front end

davoman

ASKER

Brilliant

thanks very much for that and the explanation

thanks again

davoman1

adathelad

No problem, glad I could help