Trouble with Cursors

  • Hi I think I have a problem with my Blocks [BEGIN-END] but I'm not sure. There are a number of tables prefixed with 'tmp' in the database just created by other code to produce a pair of files from each to import to MapInfo. One file contains the attributes [.mid] and the other the spatial information [.mif]. I'm using the cursor to increment through each row in each table and add a row to MidTable [attributes] and several rows to the MidTable. These are basically text files represented as rows of so I can export them using BCP. I found this was the only way to make the process fast and efficiant.

    The tables are in similarly named pairs like 'tmpFrogsLo' and 'tmpFrogsHi' and as they appear to be processed in order I SEEM to get one correct set of Mid/Mif files and one incorrect. The second has it's header missing [from the .mif file] the data seems to start a little way into the table.

    I get the error "A cursor with the name 'Table_Cursor' does not exist.". This code is being run from PowerShell as I get resource errors if I try within SSMS so the errors are sent to an OUTPUT log file, I guess it has a problem with about a million rows. As far as I can tell I process one table, throw the cursor away and re-create it at the start of the next table.

    I've removed some of the code as it was pretty long.

    USE NBNExtension

    --Create Table to hold rows

    IF Object_Id('dbo.MifTable') IS NOT NULL

    BEGIN

    Drop Table MifTable

    END

    CREATE TABLE MifTable(Row VARCHAR(500))

    GO

    IF Object_Id('dbo.MidTable') IS NOT NULL

    BEGIN

    Drop Table MidTable

    END

    CREATE TABLE MidTable(Row VARCHAR(500))

    GO

    DECLARES...@Scientific Char(80),@Common Char(80), ETC

    /* End of Table Cols */

    Declare@SQL VARCHAR(8000),

    @TableList VARCHAR(MAX), ETC

    DECLARE @MaxRows Integer;

    DECLARE @SQLString nvarchar(500);

    DECLARE @ParmDefinition nvarchar(500);

    DECLARE @String VARCHAR(MAX);

    DECLARE @Offset Integer;

    Declare @MidRow VARCHAR(500)

    SET @FilePath = '\\swt-archive\reference\MapInfo\SERC\Species\NBNExtension\'

    -- For each table Starting tmp, create a list of tables

    SELECT @TableList = COALESCE(@TableList + name + ',','')

    FROM sysobjects

    WHERE LEFT(sysobjects.name,3) = 'tmp';

    SET @Pos = PatIndex('%,%' , @TableList)

    Print @Pos

    WHILE @Pos <> 0

    BEGIN --Loop Through Table List

    SET @TableName = left(@TableList, @Pos - 1)-- Get the table name

    SET @FileName = @FilePath + @TableName + '.mif'

    SET @TableName = left(@TableList, @Pos - 1)-- Get the table name

    -- Create a dataset...

    SET @SQLString = N'SELECT @MaxRows = COUNT(*) FROM ' + @TableName

    SET @ParmDefinition = N'@MaxRows Int OUTPUT';

    EXEC sp_executesql @SQLString, @ParmDefinition, @MaxRows OUTPUT;

    /* Set up Mif file name and create with header */

    SET @MifFile = @FilePath + RIGHT(@TableName,LEN(@TableName)-3) + '.Mif'

    SET @MidFile = @FilePath + RIGHT(@TableName,LEN(@TableName)-3) + '.Mid'

    DECLARE @Counter As Integer = @MaxRows;

    DECLARE @GridLen As Integer;

    /* Set the MIF Header */

    SET @MidRow = 'Version 300'

    INSERT INTO MifTable (Row) VALUES(@MidRow) ETC..

    /* End Of MIF Header */

    EXECUTE('DECLARE Table_Cursor CURSOR FOR SELECT Scientific, Common, Site, Location, Grid, Start_Date, End_Date, Abundance, Stage, [Key], Comment, Easting, Northing, Determiner, Recorders, Other_Name, GRPrecision, Vague_Date, Taxon_Group, Survey_Name, Source, Sps_ID, Spatial_Ref_Qualifier, All_Designated, EU_Protected, EU_Priority, Red_Listed, UK_Protected, Nationally_Scarce, Amber_Birds, Red_Birds, BAP2007, LBAP2009, County_Notable, SERC_Invasive, Taxon_Rank_Key, Orders, Family FROM ' + @TableName)

    OPEN Table_Cursor;

    WHILE @Counter > 0

    BEGIN -- loop through the table

    FETCH NEXT FROM Table_Cursor INTO @Scientific, @Common, @Site, @Location, @Grid, @Start_Date, @End_Date, @Abundance, @Stage, @key, @Comment, @Easting, @Northing, @Determiner, @Recorders, @Other_Name, @GRPrecision, @Vague_Date, @Taxon_Group, @Survey_Name, @Source, @Sps_ID, @Spatial_Ref_Qualifier, @All_Designated, @EU_Protected, @EU_Priority, @Red_Listed, @UK_Protected, @Nationally_Scarce, @Amber_Birds, @Red_Birds, @BAP2007, @LBAP2009, @County_Notable, @SERC_Invasive, @Taxon_Rank_Key, @Orders, @Family;

    SET @String = '"' + RTRIM(COALESCE(@Scientific,'')) + '","' + RTRIM(COALESCE(@Common,'')) + '","' + RTRIM(COALESCE(@Site,'')) + '","' + RTRIM(COALESCE(@Location,'')) + '","' + RTRIM(COALESCE(@Grid,'')) + '","' + RTRIM(COALESCE(@Start_Date,'')) + '","' + RTRIM(COALESCE(@End_Date,'')) + '","' + RTRIM(COALESCE(@Abundance,''))+ '","' + RTRIM(COALESCE(@Stage,'')) + '","' + RTRIM(COALESCE(@Custodian,'')) + '","' + RTRIM(COALESCE(@Key,'')) + '","' + RTRIM(COALESCE(CAST(@Comment As VARCHAR(254)),'')) + '","' + RTRIM(COALESCE(@Easting,'')) + '","' + RTRIM(COALESCE(@Northing,'')) + '","' + RTRIM(COALESCE(@Determiner,'')) + '","' + RTRIM(COALESCE(CAST(@Recorders As VARCHAR(100)),'')) + '","' + RTRIM(COALESCE(@Other_Name,'')) + '","' + RTRIM(COALESCE(@GRPrecision,'')) + '","' + RTRIM(COALESCE(@Vague_Date,'')) + '","' + RTRIM(COALESCE(@Taxon_Group,'')) + '","' + RTRIM(COALESCE(@Survey_Name,'')) + '","' + RTRIM(COALESCE(@Source,'')) + '","' + RTRIM(COALESCE(@Sps_ID,'')) + '","' + RTRIM(COALESCE(@Spatial_Ref_Qualifier,'')) + '","' + RTRIM(COALESCE(CAST(@All_Designated As VARCHAR(254)),'')) + '","' + RTRIM(COALESCE(@EU_Protected,'')) + '","' + RTRIM(COALESCE(@EU_Priority,'')) + '","' + RTRIM(COALESCE(@Red_Listed,'')) + '","' + RTRIM(COALESCE(@UK_Protected,'')) + '","' + RTRIM(COALESCE(@Nationally_Scarce,'')) + '","' + RTRIM(COALESCE(@Amber_Birds,'')) + '","' + RTRIM(COALESCE(@Red_Birds,'')) + '","' + RTRIM(COALESCE(@BAP2007,'')) + '","' + RTRIM(COALESCE(@LBAP2009,'')) + '","' + RTRIM(COALESCE(@County_Notable,'')) + '","' + RTRIM(COALESCE(@SERC_Invasive,'')) + '","' + RTRIM(COALESCE(@Taxon_Rank_Key,'')) + '","' + RTRIM(COALESCE(@Orders,'')) + '","' + RTRIM(COALESCE(@Family,'')) + '"'

    INSERT INTO MidTable (Row) VALUES(@String)

    PRINT (N'Start' + @String);

    --Now write to the mif file

    SET @MidRow = 'Region 1'

    INSERT INTO MifTable (Row) VALUES(@MidRow)

    SET @MidRow = ' 5'

    INSERT INTO MifTable (Row) VALUES(@MidRow)

    SET @GridLen = LEN(@Grid)

    If (@GridLen = 4)

    BEGIN

    SET @Offset = 10000;

    END

    ELSE If (@GridLen = 5)

    BEGIN

    SET @Offset = 2000;

    END

    ELSE If (@GridLen = 6)

    BEGIN

    SET @Offset = 1000;

    END

    ELSE If (@GridLen = 8)

    BEGIN

    SET @Offset = 100;

    END

    ELSE If (@GridLen = 10)

    BEGIN

    SET @Offset = 10;

    END

    ELSE If (@GridLen = 12)

    BEGIN

    SET @Offset = 1;

    END

    SET @MidRow = STR(@Easting) + ' ' + STR(@Northing)

    INSERT INTO MifTable (Row) VALUES(@MidRow);--First coord

    SET @Calc = @Northing + @Offset

    SET @MidRow = STR(@Easting) + ' ' + STR(@Calc)

    INSERT INTO MifTable (Row) VALUES(@MidRow);--Second coord

    SET @MidRow = STR(@Easting + @Offset) + ' ' + STR(@Northing + @Offset)

    INSERT INTO MifTable (Row) VALUES(@MidRow);--Third coord

    SET @MidRow = STR(@Easting + @Offset) + ' ' + STR(@Northing)

    INSERT INTO MifTable (Row) VALUES(@MidRow);--Fourth coord

    SET @MidRow = STR(@Easting) + ' ' + STR(@Northing)

    INSERT INTO MifTable (Row) VALUES(@MidRow);--Fith coord, close region

    SET @Counter = @Counter - 1

    END -- loop through the table

    CLOSE Table_Cursor

    DEALLOCATE Table_Cursor

    SELECT @SQL = 'bcp NBNExtension..MifTable out '+@MifFile+' -c -T -S'+ @@SERVERNAME

    EXEC NBNExtension..xp_cmdshell @SQL

    SELECT @SQL = 'bcp NBNExtension..MidTable out '+@MidFile+' -c -T -S'+ @@SERVERNAME

    EXEC NBNExtension..xp_cmdshell @SQL

    DELETE FROM dbo.MifTable; -- Clear all the data.

    DELETE FROM dbo.MidTable;

    --Next table in the list

    SET @TableList = Stuff(@TableList, 1, @Pos, '')

    SET @Pos = PatIndex('%,%' , @TableList)

    END

  • This was removed by the editor as SPAM

  • Thanks I am using -c already

  • The missing table is caused by a flaw in your Coalesce statement when you assign a value to @TableList.

    Try this in a proof-of-concept database:

    CREATE TABLE tmp1 (

    C INT);

    GO

    CREATE TABLE tmp2 (

    C INT);

    GO

    CREATE TABLE tmp3 (

    C INT);

    GO

    Declare@SQL VARCHAR(8000),

    @TableList VARCHAR(MAX);

    SELECT @TableList = COALESCE(@TableList + name + ',','')

    FROM sysobjects

    WHERE LEFT(sysobjects.name,3) = 'tmp';

    PRINT @TableList;

    The problem is that, for tmp1 (in this test case), @TableList is null because it hasn't been assigned any value yet. NULL + name + ',' = NULL, so Coalesce grabs the first non-NULL value, which is an empty string in this case.

    You can change the Coalesce statement as follows:

    COALESCE(@TableList + name + ',', name + ',', '')

    Alternatively, instead of relying on a quirky-update-cursor (which is recommended against, since it's not documented behavior), and use a documented feature:

    Declare@SQL VARCHAR(8000),

    @TableList VARCHAR(MAX);

    SELECT @TableList = (SELECT name + ',' FROM sys.tables WHERE name LIKE 'tmp%' FOR XML PATH(''), TYPE).value('.[1]','varchar(8000)')

    PRINT @TableList;

    Microsoft documented that use of "For XML" as a valid way to generate a list from a column of values. It's efficient, fast, and avoids the kind of errors that you ran into here. Plus, it's not in the "could go away or change its behavior without you knowing about it" category, unlike the quirky-update-cursor you're currently using.

    I have to admit, I'm not sure why you turn it into a string and then parse it out the way you do. Why not just declare a cursor over the table names, and step through that, with the other cursor nested inside it? As long as you're already using cursors, why not be consistent on it?

    But if you are going to use the string parsing method, I recomment the For XML query for building the string.

    - Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
    Property of The Thread

    "Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon

  • Thanks GSquared the code had evolved from examples found elsewhere and my own C & Basic background. I was reluctant at first to use cursors as others say they are not good to use. I had tried several approaches before coming to this as none of the others panned out in the end. I had got closest with this. I will re-write it tomorrow taking what you suggest on board.

    Thanks

  • There are times when cursors are the best or even the only viable solution. No question about that. Generally speaking, avoid them. But in specific cases, they're your best tool.

    I'd just go with a nested cursor in this case. The big, inner cursor, is going to be the big performance issue. The loop through a list of tables is going to be negligible compared to that, and will make the code more consistent and easier to read, debug, refactor, etc. Besides, looping through tables and/or databases, is one of the uses of cursors that pretty much every DBA accepts, since other methods of looping through them are amost always less efficient. It's common practice and follows established standards.

    I didn't really look over the inner query (the cursor), so can't speak to methods of converting it to a set-based operation. Might be possible, but I don't have time right now to look at that. Was really just addressing the missing table question.

    - Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
    Property of The Thread

    "Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon

  • I believe that most of the time, you can use a WHILE loop rather than a cursor. I'm not saying that a WHILE loop is a lot better, but recently a SQL Server MVP tweeted that there may be a memory leak associated with cursors.

    The greatest enemy of knowledge is not ignorance, it is the illusion of knowledge. - Stephen Hawking

  • mtillman-921105 (7/10/2012)


    I believe that most of the time, you can use a WHILE loop rather than a cursor. I'm not saying that a WHILE loop is a lot better, but recently a SQL Server MVP tweeted that there may be a memory leak associated with cursors.

    I wouldn't doubt it. A reference would be nice if you can dig one up.

    There are no special teachers of virtue, because virtue is taught by the whole community.
    --Plato

  • This isn't from the SQL MVP I was referring to, and it's about 4 years old, but for example see the comments at the end of this MSDN article:

    http://blogs.msdn.com/b/sqlprogrammability/archive/2006/04/27/585170.aspx

    (The comments were from 2008, although the article is dated in 2006.)

    The greatest enemy of knowledge is not ignorance, it is the illusion of knowledge. - Stephen Hawking

  • Thanks. The persistent use of memory while the cursor is open is not necessarily the same as a memory leak. Of course if your system must support a large volume of cursors then memory consumption could become a bottleneck. As always, avoiding cursors in the first place where possible is probably the best defense.

    If you ever dig up that link referring to an actual memory leak I would be interested in seeing it.

    There are no special teachers of virtue, because virtue is taught by the whole community.
    --Plato

  • I understood "cursors tend to hold on to resources" to imply that they hold on to them even after they're closed and deallocated. Of course they would use resourses while they're still open.

    But you may be right, I may be reading too much into that comment.

    The greatest enemy of knowledge is not ignorance, it is the illusion of knowledge. - Stephen Hawking

  • I used to have a manager who insisted that SQL Server itself "leaked memory", because it takes memory from the server, and doesn't give it back, even if it's not still using it. The minor detail that that's by design and has nothing whatsoever to do with a "memory leak", that didn't bother him at all. Of course, the same guy thought that anyone who programs in anything except C# or Java is "just a macro programmer", was convinced that you couldn't be a real programmer if you don't know what a stack is and how to handle a mutex, and so on. Oddball opinions all over the place.

    So, yeah, cursors grab RAM resources, and then SQL Server holds onto them. That's true. But it's not a memory leak. A memory leak is when you allocate RAM addresses for storing values (like you do indirectly by defining variables), and then don't deallocate those addresses. The most common cause of this in higher level programming languages is defining a variable at too high a scope, and then "garbage cleanup" doesn't deallocate it, because the higher scope doesn't actually terminate. Cursors in SQL 2008 don't have that behavior. They may have during early testing, I don't know, but they don't in the release-versions of the software.

    If you want to test that yourself, create a non-terminating loop, and have it declare, populate, and deallocate a cursor. Fire it up and let it run for a few days. See if you end up with SQL Server crashing because of resource-starvation. If cursors "leaked", it would. If they don't, it won't.

    - Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
    Property of The Thread

    "Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon

Viewing 12 posts - 1 through 11 (of 11 total)

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