CODEWITHSUNDEEP
mysqlsqlstored-procedures
Sam ツ
Sam ツ

Reputation: 611

MySql Procedure Producing Wrong Results

I noticed a very interesting (and unexpected as well) thing yesterday. I was given a task (on production environment) to update three columns of TableA (I am changing the table and column names due to some obvious reasons) by getting all the values present in dummytable. The primary key of both the tables is column A. I know that this task was very simple and could be accomplished in several ways but I chose to write a stored procedure (given below) for that.

When the stored procedure was finished executing then it was noticed that columns B, C & statusCode were having the same values (i.e. thousands of records were having identical values in these three columns). Can someone tell me what went wrong?

1) What's wrong (or missing) in this stored procedure? (Dummy table had thousands of records as well) 2) What could be the best possible way of doing this task other than creating a stored procedure?

PS: I created (executed as well) this stored procedure on production environment using MySQL workbench and I got an exception during the execution of the procedure which stated something "Lost connection to MySQL server" but I guess since I was running this procedure on the remote machine then there was no interruption on the server while the procedure was executing.

Here is my stored procedure.

DELIMITER $$

CREATE DEFINER=`ABC`@`%` PROCEDURE `RetrieveExtractionData`()

BEGIN

DECLARE claimlisttraversed BOOLEAN DEFAULT FALSE;

DECLARE a VARCHAR(20);

DECLARE b INTEGER;

DECLARE c INTEGER;


DECLARE claimlist CURSOR FOR SELECT
`dummytable`.`A`,
`dummytable`.`B`,
`dummytable`.`C`
FROM `ABC`.`dummytable`;

DECLARE CONTINUE HANDLER FOR NOT FOUND SET claimlisttraversed = TRUE;


OPEN claimlist;

claimlistloop: LOOP

    FETCH claimlist INTO a, b, c;

    IF claimlisttraversed THEN
        CLOSE claimlist;
        LEAVE claimlistloop;
    END IF;
    UPDATE `ABC`.`TableA`
SET
`B` = b,
`C` = c,
`statuscode` = 'Sent'
WHERE `A` = a;

END LOOP claimlistloop;

END

Upvotes: 0

Views: 163

Answers (3)

KaeL
KaeL

Reputation: 3659

For your first question:

1) What's wrong (or missing) in this stored procedure? (Dummy table had thousands of records as well)

I guess you forgot to CLOSE the CURSOR. Right after you end the LOOP, you should CLOSE the CURSOR.

END LOOP claimlistloop;

CLOSE claimlist;

END

2) What could be the best possible way of doing this task other than creating a stored procedure?

Doing that in the STORED PROCEDURE should be fine. And also using CURSOR would be fine since you will just execute the procedure once (I guess because this is a production fix).

But from your question, you just want to update TableA based from the provided DummyTable. I assume that these tables have the same columns.

So I think this query is better than the CURSOR:

UPDATE TableA A
    INNER JOIN DummyTable D ON D.A = A.A
    SET A.B = D.B
        , A.C = D.C
        , A.statuscode = 'Sent';

But please try it first on a backup or dummy table. I haven't tested it yet.

Upvotes: 1

peterm
peterm

Reputation: 92785

1) What's wrong (or missing) in this stored procedure? (Dummy table had thousands of records as well)
2) What could be the best possible way of doing this task other than creating a stored procedure?

IMHO the most important thing that you're currently missing is that you don't need any cursors for that. Your whole stored procedure is one UPDATE statement. Execute it alone or wrap it in a stored procedure 

CREATE PROCEDURE RetrieveExtractionData()
UPDATE TableA a JOIN dummytable d
    ON a.a = d.a
   SET a.b = d.b, a.c = d.c, a.statuscode = 'Sent';

You don't even need to change a delimiter and use BEGIN ... END block

Here is SQLFiddle demo.

Upvotes: 1

fancyPants
fancyPants

Reputation: 51888

Forget the cursor. In fact you should never use a cursor if it's avoidable. Cursors are incredibly slow.

Simply do 

UPDATE 
yourTable yt
INNER JOIN dummyTable dt ON yt.A = dt.A
SET
yt.B = dt.B,
yt.C = dt.C;

and you're fine.

Upvotes: 1

Related Questions