Results 1 to 2 of 2
  1. #1
    Join Date
    Mar 2009
    Location
    Sydney, Australia
    Posts
    258

    Unanswered: Moved (Poclovis): Code Converted to Cursor

    Poclovis wrote:

    Hello all,

    An engineer converted the original query below into a stored procedure with cursors.

    Question: Would it have been better to just use a temp table only? The output is usually only 3-4k rows each night.

    Original Query:
    Code:
        select product_id as "GAME",
        serial_number as "PACK",
        begin_piece as "STARTING SERIAL",
        end_piece as "ENDING SERIAL",
        location_id as "LOCATION",
        ips_text_status.status_description as "STATUS"
        from inv_serial_tracking, ips_text_status
        where location_type = 2 /* FIELDREP = 2*/
        and product_id < 900
        and ips_text_status.product_type = 1
        and ips_text_status.gtms_product_id = 59
        and ips_text_status.inv_status_id = inv_serial_tracking.inv_status_id
    Converted to:

    Code:
        /*--------------------------------------------------------------------------------------*/
        /* populating the x_temp_salesrep_audit table with the data */
        /*--------------------------------------------------------------------------------------*/
    
        CREATE proc proc_build_salesrep_audit
        as
        BEGIN
        DECLARE @gtms_product_id int,
        @product_type int,
        @location_type int,
        @product_id int,
        @game int,
        @pack int,
        @begin_piece int,
        @end_piece int,
        @location int,
        @description varchar(30),
        @rec_num int
    
        select @gtms_product_id = 59,
        @product_type = 1,
        @location_type = 2,
        @product_id = 900
    
        DECLARE cur_res CURSOR for
        SELECT product_id, serial_number, begin_piece, end_piece, location_id,
        ips_text_status.status_description
        FROM inv_serial_tracking, ips_text_status
        WHERE location_type = @location_type and
        product_id < @product_id and
        ips_text_status.product_type = @product_type and
        ips_text_status.gtms_product_id = @gtms_product_id and
        ips_text_status.inv_status_id = inv_serial_tracking.inv_status_id
        ORDER BY product_id, serial_number asc
        for read only
        open cur_res
        fetch cur_res into @game,@pack, @begin_piece, @end_piece, @location, @description
    
        select @rec_num = 1
    
        while @@sqlstatus = 0
        BEGIN
    
        insert into x_temp_salesrep_audit values (@rec_num,@game,@pack, @begin_piece, @end_piece, @location, @description)
        select @rec_num = @rec_num + 1
    
        fetch cur_res into @game,@pack, @begin_piece, @end_piece, @location, @description
    
        END
        deallocate cursor cur_res
        END
    
        go

  2. #2
    Join Date
    Mar 2009
    Location
    Sydney, Australia
    Posts
    258
    1 Functionally, the two code segments are not the same; one is a pure select, the second inserts into a x_temp_salesrep_audit table. Since the selects are identical, I will assume the first code segment is an insert-select, and you have not posted the insert bit.

    2. Yes, it is a really stupid and inefficient way of loading the table, row-by-row, turning the set-oriented engine on its head. It will be slow. There is a rec_num, so he might be building a temp table for some good or bad reason. Temp tables vs cursors is not the issue, they are both two evils. Try to get the tables directly, without cursors and without temp tables.

    3 It was probably done because the insert-select of 3-4k rows did nasty things to the tran log, or held too many locks. Now that is something to be avoided, good thing to fix the code, but the method chosen is a very bad one.

    Here's another method, that maintains the set-oriented speed, and avoids hanging up the log or holding too many locks. This is good for breaking any large batch into smaller, more social (in OLTP terms) batches, it is essential for mass Deletes.
    Code:
    SET ROWCOUNT 500
    WHILE (1=1)
        BEGIN
        INSERT target
            SELECT ...
                FROM source
                WHERE source_PK > (
                SELECT MAX(target_PK)
                    FROM target
                    )
        IF @@ROWCOUNT != 500
            BREAK
        END
    SET ROWCOUNT 0
    With a tiny bit more code, it can be used for mass Updates as well.

    Try to get rid of the x_temp_salesrep_audit temp table altogether.
    Last edited by Derek Asirvadem; 09-02-09 at 12:07.
    Regards
    Derek Asirvadem (Formerly DerekA)
    Information Architect / Senior Sybase DBA
    Copyright 2009 Software Gems Pty Ltd

    I answer questions from the Original Poster only. If you have a genuine question, as the moderators have requested, start a new thread.

    http://www.softwaregems.com.au

Posting Permissions

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