Page 1 of 2 12 LastLast
Results 1 to 15 of 27
  1. #1
    Join Date
    Mar 2008
    Posts
    52

    Unanswered: Evaluate my stored procedure for paging, filtering, and sorting

    This is for SQL Server 2000. The purpose of the procedure is to return a subset of a filtered and sorted result set. The subset, filter criteria, and sort column and sort direction can be set dynamically. It uses the rowcount technique for paging.

    This would be used to drive an ASP.NET gridview which supports filtering of the data, paging, and sorting. Please let me know what improvements I can make or if you have an idea for a better solution. (I didn't put this in a vBulletin code block because personally I find two sets of scroll bars annoying, but I can if people think it's better).

    CREATE PROCEDURE dbo.Books_GetFilteredSortedSubset
    (
    -- paging
    @startRowIndex INT = 1,
    @maximumRows INT = 999999,

    -- sorting
    @sortColumn NVARCHAR(30) = 'title_id',
    @sortDirection NVARCHAR(4) = 'ASC',

    -- filtering
    @title VARCHAR(100) = NULL,
    @type VARCHAR(30) = NULL,
    @price MONEY = NULL
    )
    AS
    BEGIN

    DECLARE @sql NVARCHAR(4000)
    DECLARE @parameters NVARCHAR(4000)
    DECLARE @tableSource NVARCHAR(4000)
    DECLARE @orderByExpression NVARCHAR(4000)
    DECLARE @searchCondition NVARCHAR(4000)
    DECLARE @uniqueKey NVARCHAR(30)


    -- set the unique key used to ensure the rows are sorted deterministically
    SET @uniqueKey = 'title_id'



    -- build the FROM table source used throughout this procedure
    SET @tableSource = 'titles t
    inner join publishers p on t.pub_id = p.pub_id'




    -- build the WHERE search condition used to control filtering throughout this procedure

    SET @searchCondition = '(1 = 1)'

    IF @title IS NOT NULL
    SET @searchCondition = @searchCondition + ' AND (title LIKE ''%' + @title + '%'')'
    IF @type IS NOT NULL
    SET @searchCondition = @searchCondition + ' AND (type LIKE ''' + @type + '%'')'
    IF @price IS NOT NULL
    SET @searchCondition = @searchCondition + ' AND (price = ' + CAST(@price AS NVARCHAR) + ')'




    -- build the ORDER BY expression used to control the sorting throughout this procedure

    SET @orderByExpression = @sortColumn + ' ' + @sortDirection

    -- add uniqeKey to ORDER BY statement to ensure consistent ordering of results when @sortColumn is not unique
    IF @sortColumn <> @uniqueKey
    SET @orderByExpression = @orderByExpression + ', ' + @uniqueKey + ' ' + @sortDirection





    -- Get the column value at the position specified by @startRowIndex when the results are sorted in the desired sort order

    SET @sql = 'SET ROWCOUNT @rowcount; SELECT @start_row = ' + @sortColumn + ', @start_row_id = ' + @uniqueKey +
    ' FROM ' + @tableSource +
    ' WHERE ' + @searchCondition + ' ORDER BY ' + @orderByExpression

    PRINT @sql

    SET @parameters = '@rowcount INT, @start_row sql_variant OUTPUT, @start_row_id sql_variant OUTPUT'

    DECLARE @start_row sql_variant
    DECLARE @start_row_id sql_variant

    EXEC sp_executesql @sql, @parameters, @rowcount = @startRowIndex, @start_row = @start_row OUTPUT, @start_row_id = @start_row_id OUTPUT



    -- Get the filtered subset of results

    -- add sql to filter the results based on criteria passed in as parameters
    SET @sql = 'SET ROWCOUNT @rowcount; ' +
    'SELECT
    t.title_id,
    t.title,
    t.price,
    t.type,
    p.pub_name,
    p.city,
    p.state,
    p.country
    FROM ' + @tableSource +
    ' WHERE (' + @searchCondition + ') AND '

    -- add sql to control the starting row
    IF @sortDirection = 'ASC'
    SET @sql = @sql + '( (' + @sortColumn + ' > @start_row) OR (' +
    @sortColumn + ' = @start_row AND ' + @uniqueKey + ' >= @start_row_id) )'
    ELSE
    SET @sql = @sql + '( (' + @sortColumn + ' < @start_row) OR (' +
    @sortColumn + ' = @start_row AND ' + @uniqueKey + ' <= @start_row_id) )'

    -- add sql to control the ordering of everything
    SET @sql = @sql + ' ORDER BY ' + @orderByExpression

    PRINT @sql

    SET @parameters = '@rowcount INT, @start_row sql_variant, @start_row_id sql_variant'

    EXEC sp_executesql @sql, @parameters, @rowcount = @maximumRows, @start_row = @start_row, @start_row_id = @start_row_id


    -- Reset the rowcount for others
    SET ROWCOUNT 0

    END;
    GO

  2. #2
    Join Date
    Jul 2003
    Location
    San Antonio, TX
    Posts
    3,662
    Your @searchCondition will getchyah
    "The data in a record depends on the Key to the record, the Whole Key, and
    nothing but the Key, so help me Codd."

  3. #3
    Join Date
    Mar 2008
    Posts
    52
    Could you elaborate?

  4. #4
    Join Date
    Jul 2003
    Location
    San Antonio, TX
    Posts
    3,662
    If "title" is not null, - you'll get table or index scan.
    "The data in a record depends on the Key to the record, the Whole Key, and
    nothing but the Key, so help me Codd."

  5. #5
    Join Date
    Feb 2004
    Location
    One Flump in One Place
    Posts
    14,912
    You may be using sp_executesql as you have heard it is more efficient and secure than exec(). However, if you concatenate inputs into the SQL statement then you are bypassing the security and parameterisation efficiencies. Why not parameterise the whole statement since you clearly know how to use sp_executesql correctly?

    Also - Robert's point assumes type and\ or title are indexed - are they?
    Testimonial:
    pootle flump
    ur codings are working excelent.

  6. #6
    Join Date
    Mar 2008
    Posts
    52
    Quote Originally Posted by rdjabarov
    If "title" is not null, - you'll get table or index scan.
    But this is unavoidable in order to have partial text matching, right?

    Quote Originally Posted by pootle flump
    Also - Robert's point assumes type and\ or title are indexed - are they?
    I'm just using the Microsoft pubs database as an example. The procedure can pretty easily be adapted to any database. In the case of pubs, title is indexed but type is not.

  7. #7
    Join Date
    Jul 2003
    Location
    San Antonio, TX
    Posts
    3,662
    So, you're trying to write a reusable procedure?
    "The data in a record depends on the Key to the record, the Whole Key, and
    nothing but the Key, so help me Codd."

  8. #8
    Join Date
    Mar 2008
    Posts
    52
    I'm not trying to write a reusable procedure but, I believe, because of the nature of the problem it can easily be reused. For any result set x, it's allowing you to get a subset of the result set ordered in a certain way.

  9. #9
    Join Date
    Jul 2003
    Location
    San Antonio, TX
    Posts
    3,662
    From a programmer's standpoint, you're done. The proc works in the dev environment, and you're ready to move on to a new and exciting gig. It's the DBA who will be stuck with performance issues related to this proc, and most likely his response will be "I inherited this app, and I can't change the code. So what should I do to improve its performance?" And of course, my answer would be "The procedure was not written with scalability in mind, so without rewriting it there is nothing that can be done, you're stuck with it, so start throwing hardware at it "
    "The data in a record depends on the Key to the record, the Whole Key, and
    nothing but the Key, so help me Codd."

  10. #10
    Join Date
    Mar 2008
    Posts
    52
    Fair enough, how could it be improved?

  11. #11
    Join Date
    May 2004
    Location
    Seattle
    Posts
    1,313
    you have a lot of dynamic search conditions. may benefit from reading this:

    http://sommarskog.se/dyn-search.html

    also read his article on dynamic sql since you are using so much:

    http://sommarskog.se/dynamic_sql.html

  12. #12
    Join Date
    Mar 2008
    Posts
    52
    Quote Originally Posted by pootle flump
    ...if you concatenate inputs into the SQL statement then you are bypassing the security and parameterisation efficiencies. Why not parameterise the whole statement since you clearly know how to use sp_executesql correctly?
    Good point. I now see that I can parameterize the search conditions. However, I can't parameterize the whole statement as the ORDER BY statement won't take parameters (I know there are some workarounds using CASE but I think that will be more trouble than it's worth in this case)

  13. #13
    Join Date
    May 2008
    Posts
    3

    It is a bug

    This code has a bug in it that means it cannot return the very first row.

  14. #14
    Join Date
    May 2008
    Posts
    3

    fix

    You must add:
    SET @startRowIndex = @startRowIndex + 1

  15. #15
    Join Date
    May 2008
    Posts
    3

    Another problem

    it is not working with null values (if a column has null values)

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •