Optimize Store Proc

  • Given this table:

    CREATE TABLE [dbo].[Product](

    [ProductID] [int] IDENTITY(1,1) NOT NULL,

    [Product_Code] [nvarchar](255) NOT NULL UNIQUE,

    [Product_Qty] [int] NULL,

    CONSTRAINT pk_Product_ID PRIMARY KEY (ProductID)

    ) ON [PRIMARY]

    Can we optimize this Store Proc?

    CREATE PROCEDURE [dbo].[AlterProducts]

    AS

    BEGIN

    DECLARE @product_Id int, @product_Code nvarchar(MAX)

    DECLARE count_cursor CURSOR FOR

    SELECT ProductID, Product_Code

    FROM Product

    OPEN count_cursor

    FETCH NEXT FROM count_cursor

    INTO @product_Id, @product_Code

    WHILE @@FETCH_STATUS = 0

    BEGIN

    Print @product_Code

    UPDATE Product

    SET Product_Code = LEFT(@product_Code,@product_Id) + '-Altered'

    WHERE ProductID = @product_Id

    FETCH NEXT FROM count_cursor

    INTO @product_Id, @product_Code

    END

    CLOSE count_cursor

    DEALLOCATE count_cursor

    END

  • hi hoseam,

    May I know the exact requirement of yours please?

    this is just an update query. right?

    If it is the requirement, then why you need to use a cursor.

  • Another SQLbie (11/1/2013)


    hi hoseam,

    May I know the exact requirement of yours please?

    this is just an update query. right?

    If it is the requirement, then why you need to use a cursor.

    +100. Think 100 times before using a cursor. It seems to be pretty simple. Cursors are bad for performance and must be used ONLY in rarest case (where you can't think of any other option).

  • So you'd replace the whole thing with

    UPDATE Product SET Product_Code = Left(Product_Code,ProductID) + '-Altered';

    That's still a really weird thing to do though. What is the Left(Product_Code,ProductID) bit supposed to do?

  • Richard Warr (11/1/2013)


    So you'd replace the whole thing with

    UPDATE Product SET Product_Code = Left(Product_Code,ProductID) + '-Altered';

    That's still a really weird thing to do though. What is the Left(Product_Code,ProductID) bit supposed to do?

    Richard's right - that's a pretty weird thing to do. In addition to the whole procedure being replaced with a single update statement, three other things that jumped out at me:

    1. If the update is run more than once, it'll contain '-Altered-Altered' on the end of product_code.

    2. The length of product_code is 255 and the value will eventually become too long for the field, not even thinking about the value of ProductID. You would need to substring the whole thing.

    3. If you need to keep this in a stored procedure, using a nvarchar(max) for @product_code will lead to trouble.

    I hope this is an example meant to illustrate a point instead of actual code you're going to use.

Viewing 5 posts - 1 through 4 (of 4 total)

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