cursor not getting next values

  • I've built this cursor to build a new diagcode from all the values in the cursor.

    The cursor is working in the sense that it's looping, but it's not getting the next "code" or value in the list. What am I doing wrong?

    Note: I am selecting a list of codes. then I have declare a new variable and my hopes are to set that new variable = code1~code2~code3~etc.....

    --select * from test.dbo.matchcode

    --CREATE TABLE #tmp_matchcode

    --(

    -- [Code] [nvarchar](50) NULL,

    -- [acc_id] [nchar](10) NULL,

    -- [datetime] [nchar](10) NULL

    --) ON [PRIMARY]

    --GO

    DECLARE @diagcode VARCHAR(max) -- database name

    DECLARE @acc_id bigint

    DECLARE @newdiagcode VARCHAR(max) = '';

    DECLARE @datetime datetime

    DECLARE db_cursor CURSOR FOR

    select distinct m.code, ac.acc_id

    from acc_icd9 ac

    join medical_code m on m.id = ac.icd9_id

    join tempdb..##tmp_telcor tt on tt.[Accession ID] = ac.acc_id

    OPEN db_cursor

    FETCH NEXT FROM db_cursor INTO @diagcode, @acc_id

    WHILE @@FETCH_STATUS = 0

    BEGIN

    FETCH NEXT FROM db_cursor INTO @diagcode, @acc_id

    SET @newdiagcode = @diagcode + '~' + @newdiagcode

    SET @datetime = CURRENT_TIMESTAMP;

    USE [test]

    INSERT INTO [test].[dbo].[matchcode]

    ([Code]

    ,[acc_id]

    ,[datetime])

    VALUES

    ( @newdiagcode, @acc_id, @datetime)

    END

    CLOSE db_cursor

    DEALLOCATE db_cursor

    If anyone has any better ideas, please let me know.

    One last thing, I would like to insert the final results into a table with the @newdiagcode,acc_id, currenttimestamp. I don't know how to do this. Do I add this after I deallocate my cursor?

    --INSERT INTO [test].[dbo].[matchcode]

    -- ([Code]

    -- ,[acc_id]

    -- ,[date])

    -- VALUES

    -- ( @newdiagcode

    , @acc_id, @date)

  • Why do you want to use a cursor to do this.

    Think SET !

    you are using 3part naming to address the new matchcode object, remove "USE [test]" before statement INSERT INTO [test].[dbo].[matchcode]

    [edited]

    btw you should read the next row at the and of your coursor loop, not in the beginning because doing so will skip processing of the first row !

    • This reply was modified 2 years, 10 months ago by  Johan Bijnens.

    Johan

    Learn to play, play to learn !

    Dont drive faster than your guardian angel can fly ...
    but keeping both feet on the ground wont get you anywhere :w00t:

    - How to post Performance Problems
    - How to post data/code to get the best help[/url]

    - How to prevent a sore throat after hours of presenting ppt

    press F1 for solution, press shift+F1 for urgent solution 😀

    Need a bit of Powershell? How about this

    Who am I ? Sometimes this is me but most of the time this is me

  • Thank you for your reply.  I'm sorry for being such a beginner.  Can you give me an example of how I would use SET to acomplish what I am trying to do?

     

    I am pulling a list of values from a table and trying to concatentate them into one field with a ~ in between values.

  • Moving the fetch next statement fixed my issue.

     

    Thank you!

  • >> I've built this cursor to build a new diag_code from all the values in the cursor. <<

    I've been doing SQL for over 30 years and spent some time on ANSI/ISO Database Standards committee. I have written five cursors in all that time, and I know three of them could have been avoided if we have modern language constructs. Back in those days, the original SQL engines were built on top of existing filesystems and we had no choice but to use cursors. In short, you're doing it completely wrong and are about 30 years behind the technology.

    You also don't seem to know what a table is. By definition, it must have a key. Let's try and fix what you've got and then watch the answer fallout almost automatically.

    >> Note: I am selecting a list of codes. then I have declared a new variable and my hopes are to set that new variable = code1~code2~code3~etc.. <<

    RDBMS does not use lists; it uses sets. A set is a completed whole with no order, so there's no such concept as "next" or "prior" in this model of data processing. Furthermore, the columns in the table are all scalar values not concatenated lists.

    What you posted is actually not a table because there's no way you can have a key. The other thing I noticed is that you're storing a daytime is a character string, in a language that actually has temporal datatypes. Here is a still incorrect cleanup of what little you did post.

    CREATE TABLE Foobar_Codes

    (acc_id CHAR(10) NOT NULL,

    foobar_timestamp DATETIME2(0) NOT NULL DEFAULT CURRENT_TIMESTAMP,

    PRIMARY KEY PRIMARY KEY (acc_id, foobar_timestamp ) );

    You might want to look at ISO and other international standards. In particular, Unicode allows a subset of Latin alphabet digits and some punch punctuation marks in all the writing systems currently being used on earth. That means you probably don't need NCHAR(n) data types for encoding. Also notice that we have temporal datatypes and can default to the current timestamp.

    In one table, your international classification of disease is properly called ICD-9. Yet over to another table, it changes names. In fact, it's really hard to figure out exactly where any of your data elements are as they float around from table to table. an accession number changes into an account number

    This is why we require that you post clear, easy-to-understand DDL that follows all the basic rules. For example, there is no such thing as a generic magic universal "id" in RDBMS. By the laws of logic, it has to be the identifier of something in particular.

    >> If anyone has any better ideas, please let me know. <<

    Follow the rules of netiquette that have been in place for over 30 years on SQL forums. Post actual DDL, so we can figure out what you're doing. Want to try again?

    Please post DDL and follow ANSI/ISO standards when asking for help. 

  • ashatimjohn wrote:

    Moving the fetch next statement fixed my issue.

    Thank you!

    aaaand .... you missed the most important message !

    You may have fixed this little issue, but you are still using a CURSOR ( read "person who is cursing!" )

    Now, learn something and rework this little bit of code to a set-based statement !

    Johan

    Learn to play, play to learn !

    Dont drive faster than your guardian angel can fly ...
    but keeping both feet on the ground wont get you anywhere :w00t:

    - How to post Performance Problems
    - How to post data/code to get the best help[/url]

    - How to prevent a sore throat after hours of presenting ppt

    press F1 for solution, press shift+F1 for urgent solution 😀

    Need a bit of Powershell? How about this

    Who am I ? Sometimes this is me but most of the time this is me

  • ashatimjohn wrote:

    Thank you for your reply.  I'm sorry for being such a beginner.  Can you give me an example of how I would use SET to acomplish what I am trying to do?

    I am pulling a list of values from a table and trying to concatentate them into one field with a ~ in between values.

    https://ask.sqlservercentral.com/questions/34340/diff-between-cursor-based-approch-and-set-based.html

  • There are legitimate reasons to use a cursor, including using one to dynamically generate SQL to be executed by sp_executesql.

    We ran into this issue with the cursor not moving forward through the recordset. The first record only would be acted upon, and the fetch status was -1 after the fetch next at the bottom of the loop.

    Solution A: we found that putting an ORDER BY on the original cursor definition took care of our specific instance of the issue.

    DECLARE curFunction CURSOR LOCAL
    FOR
    SELECT CONCAT (
    'DROP FUNCTION IF EXISTS ['
    , r.ROUTINE_SCHEMA
    , '].['
    , r.ROUTINE_NAME
    , ']'
    ) ObjectName
    FROM OurDatabase.INFORMATION_SCHEMA.ROUTINES r
    WHERE r.ROUTINE_TYPE = 'FUNCTION'
    ORDER BY r.ROUTINE_NAME

    Solution B: We also found that putting the contents of the select in a temporary table and using that to populate the cursor worked. We suspect it might be because of an implicit order by.

    DROP TABLE IF EXISTS #FN
    SELECT CONCAT (
    'DROP FUNCTION IF EXISTS ['
    , r.ROUTINE_SCHEMA
    , '].['
    , r.ROUTINE_NAME
    , ']'
    ) ObjectName
    INTO #FN
    FROM OurDatabase.INFORMATION_SCHEMA.ROUTINES r
    WHERE r.ROUTINE_TYPE = 'FUNCTION'
    AND r.ROUTINE_CATALOG = 'Sample'

    DECLARE curFun CURSOR LOCAL
    FOR
    SELECT ObjectName
    FROM #FN

    • This reply was modified 10 months, 3 weeks ago by  LJMoss. Reason: scrubbed of identifiers
  • LJMoss wrote:

    There are legitimate reasons to use a cursor..

    Just an opinion here...

    While I agree with the statement quoted above (because "It Depends" is always a consideration), I've found that a lot of people legitimize the use of a Cursor and other forms of RBAR simply because they don't know how to do it any other way.

    For the original problem on this post, which is in an SQL Server 2019 forum, the use of the following functionality should do the trick without a Cursor, While Loop, or any other form of RBAR.

    https://learn.microsoft.com/en-us/sql/t-sql/functions/string-agg-transact-sql

    Prior to 2017 (when STRING_AGG() was first introduced), there was (and still is) and effective method of doing the same thing using FOR XML PATH.

    The real problem with this thread is the OP assumed it could only be done with a Cursor and so asked about how to fix the cursor instead of how to solve the actual problem.  As a result, he didn't post any "Readily Consumable" test data (see the article at the first link in my signature line below for one of many ways to do that).

     

    --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)

Viewing 9 posts - 1 through 8 (of 8 total)

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