Need help replacing a cursor with set-based code (if possible!)

  • I have a problem, and the only solution that I’ve found requires me to use a cursor. It’s for our 3rd party ERP system (SQL 2000), so I can’t alter the table structures or front-end code. This particular ERP segment is for city government to issue construction permits and conduct inspections. The form for scheduling inspections or entering results lets me enter multiple lines of text comments for any given inspection.

    The problem is that they store the comments as multiple lines of text: you enter it into a text box, but the front-end code breaks it down into multiple lines when it stores it in the table. There are two tables involved: a header table that contains a tag indicating what table/screen set the comment is from, the inspection number, and the internal record number to link to the actual comment text. The comment detail table has the internal record number, a line number, and the actual text.

    I need to ‘reconstitute’ that comment text line to print it properly in Crystal. I cobbled together a cursor-based solution based on code from this article, then wrapped all that into a function that is passed a key value when called from a view.

    So here’s the code that I have right now:

    [font="Courier New"]--table defs: comment headers

    CREATE TABLE [dbo].[sptexthd] (

    [spth_prog] [char] (8) NULL ,

    [spth_idxval] [char] (50) NULL ,

    [spth_keyno] [int] NOT NULL )

    GO

    --comment details

    CREATE TABLE [dbo].[sptextdt] (

    [sptd_keyno] [int] NULL ,

    [sptd_lineno] [smallint] NULL ,

    [sptd_text] [char] (70) NULL)

    GO

    CREATE UNIQUE INDEX [sptexthd1]

    ON [dbo].[sptexthd]([spth_prog], [spth_idxval])

    WITH FILLFACTOR = 90

    GO

    CREATE UNIQUE INDEX [sptextdt1]

    ON [dbo].[sptextdt]([sptd_keyno], [sptd_lineno])

    WITH FILLFACTOR = 90

    GO

    --data loads

    INSERT sptexthd

    SELECT 'INSPECTN', '000000011', 2661 UNION ALL

    SELECT 'INSPECTN', '000000012', 2662 UNION ALL

    SELECT 'INSPECTN', '000000013', 2663 UNION ALL

    SELECT 'INSPECTN', '000000029', 2659 UNION ALL

    SELECT 'INSPECTN', '000000031', 2660

    INSERT sptextdt

    SELECT 2661, 1, 'req by Wayne @ 575-642-5555.' UNION ALL

    SELECT 2662, 1, 'req by Jose Jiminez @ 877-BASEBALL. 01/10/08: GATE WAS LOCKED ON ARRI' UNION ALL

    SELECT 2662, 2, 'VAL, RESCHEDULE. Roger Ramjet: he''s our leader, fighting for our nati' UNION ALL

    SELECT 2662, 3, 'on. For his adventures just be sure to stay tuned to this station.' UNION ALL

    SELECT 2663, 1, 'req by S. Clause, 1-800-FTHRXMAS.' UNION ALL

    SELECT 2659, 1, 'APPROVED PLANS NOT ON SITE' UNION ALL

    SELECT 2660, 1, 'RQSTED BY TOM TAYLOR'

    --function/cursor def

    create function fnGetInspectionCommentText (@HeaderKey int)

    RETURNS varchar(8000)

    AS

    BEGIN

    DECLARE @HeaderID int, @CommentText varchar(8000)

    SET @CommentText = ''

    DECLARE crs_Headers CURSOR

    FOR SELECT spth_keyno

    FROM sptexthd

    WHERE spth_prog = 'INSPECTN'

    and spth_keyno = @HeaderKey

    OPEN crs_Headers

    FETCH NEXT FROM crs_Headers INTO @HeaderID

    WHILE @@FETCH_STATUS = 0

    BEGIN

    SELECT @CommentText = @CommentText + sptd_text

    FROM sptextdt dt

    WHERE dt.sptd_keyno = @HeaderID

    FETCH NEXT FROM crs_Headers INTO @HeaderID

    END

    CLOSE crs_Headers

    DEALLOCATE crs_Headers

    RETURN rtrim(@CommentText)

    END

    --view def that calls function/cursor

    --create view vwInspectionLog as

    select distinct spth_keyno as InspectionNum,

    dbo.fnGetInspectionCommentText(spth_keyno) as Comments

    from sptexthd hd

    join sptextdt dt

    on spth_keyno = sptd_keyno[/font]

    This is the result of running the mess:

    [font="Courier New"]InspectionNum Comments

    ------------- --------------------------------------------------------------------

    2659 APPROVED PLANS NOT ON SITE

    2660 RQSTED BY TOM TAYLOR

    2661 req by Wayne @ 575-642-5555.

    2662 req by Jose Jiminez @ 877-BASEBALL. 01/10/08: GATE WAS LOCKED ON ARRIVAL, RESCHEDULE. Roger Ramjet: he's our leader, fighting for our nation. For his adventures just be sure to stay tuned to this station.

    2663 req by S. Clause, 1-800-FTHRXMAS.[/font]

    The final reporting view shown here is strictly representative, the actual view joins several more tables in. The report that will be calling this will be run once or twice a day normally and produces around 250 records. Aside from permits, the header and detail tables contain a lot of records as they store these comments for lots of places within our ERP system. Each inspection has at least one comment line, it will probably have no more than 3 records in the detail table. This is strictly a guesstimate as this is not yet a production system.

    I haven’t had any luck writing a set-based solution, I think mainly because the different inspection numbers need to form a virtual line break between the comments, and I've probably missed a blindingly obvious set solution, but I just can't see it. Considering the specificity of the report view in that it calls a fairly small set of records and the cursor only deals with one inspection header record plus a small number of records from the detail table, and the fact that it is only run once or twice a day, I think that the performance hit of the cursor might be acceptable, but if there’s a better way to do it, I’m certainly open to suggestions.

    (Frankly, I’m quite happy that I was able to even get a cursor to rejoin the lines! I may have over-complicated the cursor, but it's my first, so be gentle. :P)

    -----
    [font="Arial"]Knowledge is of two kinds. We know a subject ourselves or we know where we can find information upon it. --Samuel Johnson[/font]

  • You can use a function with no cursor for this.

    create function fn_GetComments (@keyno int)

    returns varchar(8000)

    as

    begin

    declare @comment varchar(8000)

    select @comment = coalesce(@comment,'') + Rtrim(sptd_text) from sptextdt

    where sptd_keyno = @keyno

    if @comment is Null

    set @comment = ''

    return @comment

    end

    go

    select spth_keyno InspectionNum , dbo.fn_GetComments(spth_keyno) comment

    from sptexthd

    If you have access to SQL 2005, you can do it without a function. Look at this thread for Jeffs answer.

    Nicely asked btw.

    HTH

    Dave J


    http://glossopian.co.uk/
    "I don't know what I don't know."

  • With SQL2K, the only way to avoid cursor (or 'hidden' cursor in a function) is to accept a non-dynamic query where the number of lines of text queried is capped.

    Here is a set-based solution that returns a max of 5 lines of text. Join this view to the header table. Of course, there is the problem of it not being dynamic and therefore unable to cope with more than 5 lines of text. Hence the indicator column that confirms if you really included all lines. Not very elegant.

    Create View LineTextConcatenation

    As

    Select

    d1.sptd_keyno,

    d1.sptd_lineno +

    IsNull(d2.sptd_lineno, '') +

    IsNull(d3.sptd_lineno, '') +

    IsNull(d4.sptd_lineno, '') +

    IsNull(d5.sptd_lineno, '') As ConcatenatedText,

    Case When d5.sptd_lineno Is Null Then 'Y' Else 'N' End As ConfirmedEndOfText

    From sptextdt As d1

    Left Join sptextdt As d2

    on (d1.sptd_keyno = d2.sptd_keyno And d2.sptd_lineno = d1.sptd_lineno + 1)

    Left Join sptextdt As d3

    on (d1.sptd_keyno = d3.sptd_keyno And d3.sptd_lineno = d1.sptd_lineno + 2)

    Left Join sptextdt As d4

    on (d1.sptd_keyno = d4.sptd_keyno And d4.sptd_lineno = d1.sptd_lineno + 3)

    Left Join sptextdt As d5

    on (d1.sptd_keyno = d5.sptd_keyno And d5.sptd_lineno = d1.sptd_lineno + 4)

  • You can do something with the help of a temp table:

    --now for the new stuff

    create table #runningcomments(keyno int,line_no smallint,comment varchar(70),runComments varchar(8000))

    create unique clustered index UC_runncomments on #runningcomments(keyno,line_no)

    insert #runningcomments(keyno,line_no,comment)

    select sptd_keyno,sptd_lineno,sptd_text from sptextdt

    --now do the running stuff

    declare @prevID int

    declare @runText varchar(8000)

    declare @dummyText varchar(8000)

    declare @dummyID int

    select @previd=0,@runtext=''

    update #runningcomments

    set runcomments=case when keyno=@previd then @runtext+char(13)+char(10)+comment else comment end,

    @runtext=case when keyno=@previd then @runtext+comment else comment end,

    @dummyText=@runtext,

    @previd=keyno,

    @dummyID=@prevID

    from #runningcomments with (INDEX(UC_runncomments),TABLOCk) OPTION (MAXDOP 1)

    select keyno,line_no,comment from #runningcomments

    select keyno,max(runcomments) commentary from #runningcomments group by keyno

    select spth_prog,

    spth_idxval,

    keyno,max(runcomments) commentary

    from sptexthd inner join

    #runningcomments on spth_keyno=keyno

    group by spth_prog,

    spth_idxval,

    keyno

    drop table #runningcomments

    Of course - considering how small you think the recordset will be on a daily basis - the cursor might actually be FASTER.

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • David Jackson (1/15/2008)


    You can use a function with no cursor for this.

    create function fn_GetComments (@keyno int)

    returns varchar(8000)

    as

    begin

    declare @comment varchar(8000)

    select @comment = coalesce(@comment,'') + Rtrim(sptd_text) from sptextdt

    where sptd_keyno = @keyno

    ...

    That's exactly what I was aiming for, I just couldn't get it to work quite right. I think I got hung up on the syntax, couldn't get it to work, made the cursor, and never thought about plugging the syntax into a function. And I'm quite a fan of functions. Oh, well. Thanks much, David!

    Another one of those 'bloody obvious' solutions once you see it! :hehe:

    I spent a good amount of time putting together my original post, it's too complicated a system not to include enough detail so that people can duplicate what I had. I've seen far too many posts here where you had to drag every last important detail out of the poster! 😛

    -----
    [font="Arial"]Knowledge is of two kinds. We know a subject ourselves or we know where we can find information upon it. --Samuel Johnson[/font]

  • Wayne - insert Char(13)+char(10) ahead of the RTRIM if you want to have the line feeds...They'll show up when you inspect it outside of QA or SSMS.

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • One more thing, you will want an Order by sptd_lineno in the function to guarantee they come out in the right order.

    Another one of those 'bloody obvious' solutions once you see it!

    Aren't they always 😀

    Dave J


    http://glossopian.co.uk/
    "I don't know what I don't know."

  • David Jackson (1/16/2008)


    One more thing, you will want an Order by sptd_lineno in the function to guarantee they come out in the right order.

    I've taken to removing order by's in views since they're kinda wonky in 2005 and rely on Crystal or Access for my sort order, I used to do Top 100 Percent with an order by in views, but no more. The detail table has a PK on record number/line number, so it should be OK: still, you never know when a patch or service pack will screw things up, so I'll definitely do that -- better to add a line of code than to have the report blow up later.

    -----
    [font="Arial"]Knowledge is of two kinds. We know a subject ourselves or we know where we can find information upon it. --Samuel Johnson[/font]

  • Wayne - he's talking about having the ORDER BY in the select within the function. Otherwise - you have no guarantees that it will concatenate the lines in the correct order.

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • Matt Miller (1/16/2008)


    Wayne - he's talking about having the ORDER BY in the select within the function. Otherwise - you have no guarantees that it will concatenate the lines in the correct order.

    Yeah, I caught that, Matt, and modified the function. I'm confident that the ERP software stores them, at least initially, in line number order when it comes to order of insertion, but who knows what it does when those comments are modified.

    -----
    [font="Arial"]Knowledge is of two kinds. We know a subject ourselves or we know where we can find information upon it. --Samuel Johnson[/font]

  • Wayne West (1/15/2008)


    I spent a good amount of time putting together my original post, it's too complicated a system not to include enough detail so that people can duplicate what I had. I've seen far too many posts here where you had to drag every last important detail out of the poster! 😛

    Great post, Wayne... help spread the word with the following URL, please...

    http://www.sqlservercentral.com/articles/Best+Practices/61537/

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • PW (1/15/2008)


    With SQL2K, the only way to avoid cursor (or 'hidden' cursor in a function) is to accept a non-dynamic query where the number of lines of text queried is capped.

    Heh... I'm thinking that's not quite true, PW... see David Jackson's post. Only limit is the 8K barrier in SS2000.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • David Jackson (1/15/2008)


    You can use a function with no cursor for this...

    Nicely done, Dave...

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • Cheers Jeff.

    (I'm replying to test my newly created sig) 😛

    Edit: How do I get a line in? Hang on... Got it 🙂

    Dave J


    http://glossopian.co.uk/
    "I don't know what I don't know."

  • >>Heh... I'm thinking that's not quite true, PW... see David Jackson's post. Only limit is the 8K barrier in SS2000.

    Right, it is dynamic, but it doesn't avoid a cursor or 'hidden cursor'.

    The function contains a SELECT and has a parameter to be passed from the outer SELECT (see op's sample resultset - this is to be used in a set, not a single-record query)

    SELECT

    InspectionNum,

    dbo.fn_GetComments(keyno) As Comments

    FROM ...

    Doesn't that essentially mean a "hidden cursor" ? A cursor isn't declared explicitly, but a SELECT inside a function called from a SELECT is essentially RBAR (tm).

    Not saying it's wrong, it's an elegant way to solve the problem, but it has performance implications if the resultset is large.

Viewing 15 posts - 1 through 15 (of 18 total)

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