Subject Re: [firebird-support] Problem with Stored Procedure Returns
Author Helen Borrie
At 10:57 AM 14/10/2003 +1000, you wrote:
>Hi,
>
>With have an intermittent problem with a stored procedure. We're using
>Firebird 1.03 on Windows NT. This procedure gets called about 5000 times
>a day and works fine except for roughly 1 or 2 calls per day where we
>get the error:
>
>Dynamic SQL Error
>SQL error code = -84
>procedure GET_SEQUENCENEXTKEYS does not return any values
>At line 1, column 15
>
>This make no sense to me. The code for the stored procedure is
>
>COMMIT WORK;
>SET AUTODDL OFF;
>SET TERM ^ ;
>
>/* Stored procedures */
>
>CREATE PROCEDURE "GET_SEQUENCENEXTKEYS"
>(
> "KEYPREFIX" CHAR(30),
> "NEXTKEYINCREMENT" INTEGER
>)
>RETURNS
>(
> "THENEXTKEY" INTEGER
>)
>AS
>BEGIN EXIT; END ^
>
>
>ALTER PROCEDURE "GET_SEQUENCENEXTKEYS"
>(
> "KEYPREFIX" CHAR(30),
> "NEXTKEYINCREMENT" INTEGER
>)
>RETURNS
>(
> "THENEXTKEY" INTEGER
>)
>AS
>DECLARE VARIABLE TheLockFlag INT;
>DECLARE VARIABLE IncrementedNextKey INT;
>BEGIN

Start by initialising your variables !!!!

> BEGIN
> SELECT NextKey, LockFlag FROM System_SequenceNextKey WHERE Prefix =
>:keyPrefix
> INTO :TheNextKey, :TheLockFlag;

It will never go into this block because writers never block readers
(unless you are using SNAPSHOT TABLE STABILITY, of course)

> WHEN GDSCODE deadlock, GDSCODE lock_conflict DO
> TheLockFlag = 1;
> END
> IF (TheLockFlag = 1) THEN BEGIN
> TheNextKey = -1; <--- value is never assigned
> SUSPEND;
> EXIT; <--- doesn't happen
> END

So here is where the action begins

> BEGIN
> IncrementedNextKey = TheNextKey + NEXTKEYINCREMENT;
> UPDATE System_SequenceNextKey SET NextKey=:IncrementedNextKey WHERE
>Prefix = :keyPrefix;
> WHEN GDSCODE deadlock, GDSCODE lock_conflict DO BEGIN
> TheNextKey = -1;

Yup, but what happens if the GDSCODE is unique_key_violation?

> SUSPEND;
> EXIT;
> END
> END
> BEGIN
> /* Worth checking but could fail if someone gets in before us. */

Wrong assumption. You are still inside the same transaction. You haven't
committed anything yet. In SNAPSHOT, you can only see what you saw to
begin with plus your own pending update; in READCOMMITTED the update would
have already failed if someone already committed the same number.

> SELECT NextKey FROM System_SequenceNextKey WHERE Prefix = :keyPrefix
> INTO :IncrementedNextKey;
> WHEN GDSCODE deadlock, GDSCODE lock_conflict DO BEGIN
> TheNextKey = -1;
> SUSPEND;
> EXIT;
> END
> END
> IF (IncrementedNextKey <> (TheNextKey + NEXTKEYINCREMENT)) THEN BEGIN
> TheNextKey = -1;
> SUSPEND;
> EXIT;
> END
> SUSPEND;
>END
> ^
>
>SET TERM ; ^
>COMMIT WORK;
>SET AUTODDL ON;

Basically, you're not getting a value back on the occasions that your logic
never goes anywhere to get the value set. You need to initialise the
return value to -1 to cover the cases where that will occur, not set them
inside blocks that might never be visited.

I'm really curious as to why you're doing this (multi-user-risky) thing
instead of using generators (which are transaction-independent)...and why
(apparently) you're trying to achieve a no-gaps sequence for primary
keys. There are safe (and quite complicated) techniques to get no-gaps
sequences for things like document numbers and serial numbers but **keys**
should be semantically blind and surely not subjected to this sort of lottery.

heLen