SQL Server Performance

Couple of SP/Transaction advices needed.

Discussion in 'SQL Server 2005 T-SQL Performance Tuning' started by p3tar, Dec 18, 2007.

  1. p3tar New Member

    First of all I wanna say hi to everyone. HI[:)]
    I am building a quite large web application with multi language support and I am trying to avoid any possible concurrency in my database so I use transactions in my insert, update, delete SPs. Can someone advise me witch one of the two listed SPs bellow have better approach for deleting a record.
    In my database I have Pages table with small number of rows and low traffic but I'm pretending otherwise, I'm assuming that table contains large amount of rows and have low to medium traffic, because of the approach to other high traffic tables...
    Table:

    PageID int
    LanguageID int
    Title nvarchar(255)
    ListOrder int
    IsFrontPage bit
    CreatedDate datetime
    CreatedBy nvarchar(50)
    ModifiedDate datetime
    ModifiedBy nvarchar(50)
    Only one page per language can be front page. Front pages always have list order set to 0.
    SP1:
    CREATE PROCEDURE [dbo].[DeletePage]
    (
    @PageID int,
    @EnableXactAbort bit
    )

    AS
    BEGIN
    SET NOCOUNT ON
    IF(@EnableXactAbort = 1)
    SET XACT_ABORT ON
    ELSE
    SET XACT_ABORT OFF

    DECLARE @LanguageID int
    DECLARE @ListOrder int
    DECLARE @ListOrderCount int
    DECLARE @IsFrontPage bit
    SET @LanguageID = NULL
    SET @ListOrder = NULL
    SET @ListOrderCount = NULL
    SET @IsFrontPage = NULL

    DECLARE @TranStarted bit
    DECLARE @ErrorCode int
    SET @TranStarted = 0
    SET @ErrorCode = 0

    IF(@@TRANCOUNT = 0)
    BEGIN
    BEGIN TRANSACTION DeletePage
    SET @TranStarted = 1
    END
    ELSE
    SET @TranStarted = 0

    SELECT @LanguageID = LanguageID,
    @IsFrontPage = IsFrontPage ,
    @ListOrder = ListOrder FROM dbo.Pages WITH (UPDLOCK) WHERE PageID = @PageID
    SELECT @ListOrderCount = COUNT(ListOrder) FROM dbo.Pages WHERE LanguageID = @LanguageID

    IF((@LanguageID IS NULL) OR (@ListOrder IS NULL) OR (@IsFrontPage IS NULL) OR (@ListOrderCount IS NULL))
    BEGIN
    SET @ErrorCode = 1
    GOTO Cleanup
    END

    DELETE FROM dbo.Pages
    WHERE PageID = @PageID

    IF(@@ERROR <> 0)
    BEGIN
    SET @ErrorCode = -1
    GOTO Cleanup
    END

    IF(@IsFrontPage = 0)
    BEGIN
    UPDATE dbo.Pages
    SET ListOrder = (ListOrder - 1)
    WHERE ((ListOrder > @ListOrder) AND (ListOrder <= @ListOrderCount)) AND (LanguageID = @LanguageID)
    END

    IF(@@ERROR <> 0)
    BEGIN
    SET @ErrorCode = -1
    GOTO Cleanup
    END

    IF(@TranStarted = 1)
    BEGIN
    SET @TranStarted = 0
    COMMIT TRANSACTION DeletePage
    END

    Cleanup:
    IF(@TranStarted = 1)
    BEGIN
    SET @TranStarted = 0
    RAISERROR('Page delete failed', 16, 1)
    ROLLBACK TRANSACTION DeletePage
    END

    RETURN @ErrorCode
    END
    SP2:

    CREATE PROCEDURE [dbo].[DeletePage]
    (
    @LanguageID int,
    @PageID int,
    @EnableXactAbort bit
    )

    AS
    BEGIN
    SET NOCOUNT ON
    IF(@EnableXactAbort = 1)
    SET XACT_ABORT ON
    ELSE
    SET XACT_ABORT OFF

    DECLARE @ListOrder int
    DECLARE @ListOrderCount int
    DECLARE @IsFrontPage bit
    SET @ListOrder = NULL
    SET @ListOrderCount = NULL
    SET @IsFrontPage = NULL
    SELECT @IsFrontPage = IsFrontPage,
    @ListOrder = ListOrder FROM dbo.Pages WITH (UPDLOCK) WHERE PageID = @PageID
    SELECT @ListOrderCount = COUNT(1) FROM dbo.Pages WHERE LanguageID = @LanguageID

    IF((@ListOrder IS NULL) OR (@IsFrontPage IS NULL) OR (@ListOrderCount IS NULL))
    RETURN(-1)
    DECLARE @TranStarted bit
    DECLARE @ErrorCode int
    SET @TranStarted = 0
    SET @ErrorCode = 0

    IF(@@TRANCOUNT = 0)
    BEGIN
    BEGIN TRANSACTION DeletePage
    SET @TranStarted = 1
    END
    ELSE
    SET @TranStarted = 0
    IF(@IsFrontPage = 0)
    BEGIN
    UPDATE dbo.Pages
    SET ListOrder = (ListOrder - 1)
    WHERE ((ListOrder > @ListOrder) AND (ListOrder <= @ListOrderCount)) AND (LanguageID = @LanguageID)
    END

    IF(@@ERROR <> 0)
    BEGIN
    SET @ErrorCode = -1
    GOTO Cleanup
    END

    DELETE FROM dbo.Pages
    WHERE PageID = @PageID

    IF(@@ERROR <> 0)
    BEGIN
    SET @ErrorCode = -1
    GOTO Cleanup
    END

    IF(@TranStarted = 1)
    BEGIN
    SET @TranStarted = 0
    COMMIT TRANSACTION DeletePage
    END

    Cleanup:
    IF(@TranStarted = 1)
    BEGIN
    SET @TranStarted = 0
    RAISERROR('Page delete failed', 16, 1)
    ROLLBACK TRANSACTION DeletePage
    END

    RETURN @ErrorCode
    END
    SP2 is the result of reading a bunch of SP tuning articles, I haven't got the time to test it well. I moved @LanguageID outside the SP because there is a very very small chance that someone is going to update page I want to delete to another language seconds before I press a delete button, but I left @ListOrder and @IsFrontPage assuming there's a slightly bigger chance that error will occur in listorder column arrangement(1,2,3,4,4,5) if someone deletes a page from table before I press delete button and provide wrong listorder number or maybe updates page I want to delete to be a front page. I also moved selecting of @ListOrder and @IsFrontPage parameters out of the transaction for shorter transaction and moved query with higher chance for breaking transaction to beginning of the transaction. Any advice is accepted. I apologize if I made some spelling mistakes, English is not my mother language. Thanks

  2. satya Moderator

    WElcome to the forum!
    What kind of table relationship you have for such languague ID with other tables, I don't understand the issue completely but would like to know more about it.
  3. p3tar New Member

    LanguageID is the starting point, something like "ApplicationID". Every parent table have LanguageID column and there is no relationship to other tables by LanguageID. It's more a performance/concurency question than issue. Both SPs work well. I made SP2 after reading some tuning articles, so I kinda wanted to see other people opinion about my tuning of SP1 to SP2.

    EDIT: The main thing that is bothering me is should I pass parameters like @LanguageID, @ListOrder and @IsFrontPage from application so I dont have to get them inside SP and doing so I'm getting better performance from my SP, but then I'm risking a possible list order column miss numbering etc.
  4. p3tar New Member

    Another question...I have Articles table that could grow quite large. One of the columns in my Articles table is the "IsArchived" column witch is indicating whether or not article is archived. Would it be a good practice to create a separate table called, for example ArticlesArchive and store archived articles in that table. Thanks
  5. ranjitjain New Member

    If you are using SQL 2005, read about TRY..CATCH block in BOL to trap the error and in catch block perform the cleanup tasks.
  6. p3tar New Member

    [quote user="ranjitjain"]
    If you are using SQL 2005, read about TRY..CATCH block in BOL to trap the error and in catch block perform the cleanup tasks.
    [/quote]
    Thanks ranjitjain.
    One more question...if I'm calling/executing a stored procedure from a stored procedure using EXEC, and let's say that the procedure I'm executing is inside transaction statement of the parent procedure, do I have to make transaction block/logic for that procedure also or not?
  7. p3tar New Member

    [:)]....and last couple of questions regarding to TRY/CATCH block. I took ranjitjain advice and studied a little on TRY/CATCH block and I can say I really like it. So...yes...questions. I understand that RAISERROR with severity > 11 in TRY block will force execution to jump to CATCH block, so... is using RAISERROR in CATCH block same as PRINT, for gathering error info? And another, I'm a bit confused about XACT_STATE(), is it good practice to commit transaction in CATCH block if XACT_STATE() = 1.What kind of errors would have to happen in TRY block, forcing a jump to CATCH block but leaving my transaction in commitable state?. Thanks once again.
    EDIT: One more, when I use DECLARE @SomeVariable someDateType, is the variable value NULL by default or I have to SET it to null, or that depends on DataType?

  8. satya Moderator

    Make them NULL by default, as it is a good practice in fetching results.
  9. p3tar New Member

    So yust to make sure I understand...., after I declare a variable SETing it to null for fetching results is a good practice. Something like:
    DECLARE @variable1,
    @variable2
    SET @variable1 = NULL;
    SET @variable2 = NULL;

    SELECT @variable1 = xxxxxx,
    @variable2 FROM .......
    IF(@variable1 IS NULL or @variable2 IS NULL)
    .
    .
    .
    UPDATE:
    After reading a bunch of online tutorials and articles on TRY/CATCH and other new SQL 2005 new features and couple of hours of testing, here is the rewriten SP2 from above.
    New MSSQL2005ish SP[:D]:
    CREATE PROCEDURE [dbo].[DeletePage]
    (
    @PageID uniqueidentifier,
    @EnableXactAbort bit
    )

    AS
    BEGIN
    SET NOCOUNT ON;
    IF(@EnableXactAbort = 1)
    SET XACT_ABORT ON;
    ELSE
    SET XACT_ABORT OFF;

    DECLARE @LanguageID int,
    @ListOrder int,
    @ListOrderCount int,
    @IsFrontPage bit

    SELECT @LanguageID = LanguageID,
    @IsFrontPage = IsFrontPage,
    @ListOrder = ListOrder FROM dbo.Pages WHERE PageID = @PageID
    SELECT @ListOrderCount = COUNT(1) FROM dbo.Pages WITH (UPDLOCK) WHERE LanguageID = @LanguageID

    IF((@LanguageID IS NULL) OR (@ListOrder IS NULL) OR (@IsFrontPage IS NULL) OR (@ListOrderCount IS NULL))
    RETURN -1;

    BEGIN TRY
    IF(@@TRANCOUNT = 0)
    BEGIN TRANSACTION DeletePage;

    IF(@IsFrontPage = 0)
    BEGIN
    UPDATE dbo.Pages
    SET ListOrder = (ListOrder - 1)
    WHERE ((ListOrder > @ListOrder) AND (ListOrder <= @ListOrderCount)) AND (LanguageID = @LanguageID)
    END

    DELETE FROM dbo.Pages
    WHERE PageID = @PageID
    COMMIT TRANSACTION DeletePage;
    END TRY
    BEGIN CATCH
    EXECUTE dbo.DisplayError;

    IF(@@TRANCOUNT > 0)
    BEGIN
    IF(XACT_STATE() <> 0)
    ROLLBACK TRANSACTION DeletePage;
    END

    EXECUTE dbo.CaptureError;
    RETURN ERROR_NUMBER();
    END CATCH
    END
    Much cleaner. Thanks again ranjitjain for the TRY/CATCH hint. I removed SETing local variables to null. I found on couple articles/tutorials online with something like "...before assignment, the value of each variable is null. It is also possible to explicitly set the value of each variable to null..." so I dont see the point in SETing it to null or maybe I'm wrong.

Share This Page