Select FOR UPDATE gives duplicate key error

Question

I am using SELECT...FOR UPDATE to enforce a unique key. My table looks like:

CREATE TABLE tblProductKeys (
  pkKey varchar(100) DEFAULT NULL,
  fkVendor varchar(50) DEFAULT NULL,
  productType varchar(100) DEFAULT NULL,
  productKey bigint(20) DEFAULT NULL,
  UNIQUE KEY pkKey (pkKey,fkVendor,productType,productKey)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

So rows might look like:

{'Ace-Hammer','Ace','Hammer',121}, 
{'Ace-Hammer','Ace','Hammer',122},
... 
{'Menards-Hammer','Menards','Hammer',121},
...

So note that 'Ace-Hammer' and 'Menards-Hammer' can have the same productKey, only the product+key combination needs to be unique. The requirement that it is an integer defined in this way is organizational, I don't think this is something I can do with auto_increment using innoDb, but hence the question.

So if a vendor creates a new version of an existing product, we give it a distinct key for that vendor/product combination (I realize the pkKey column is redundant in these examples).

My stored procedure is like:

CREATE PROCEDURE getNewKey(IN vkey varchar(50),vvendor varchar(50),vkeyType varchar(50)) BEGIN
start transaction;

set @newKey=(select max(productKey) from tblProductKeys where pkKey=vkey and fkVendor=vvendor and productType=vkeyType FOR UPDATE);
set @newKey=coalesce(@newKey,0);
set @newKey=@newKey+1;
insert into tblProductKeys values (vkey,vclient,vkeyType,@newKey);

commit;
select @newKey as keyMax;
END

That's all! During periods of heavy use, (1000s of users), I see: Duplicate entry 'Ace-Hammer-Ace-Hammer-44613' for key 'pkKey'.

I can retry the transaction, but this is not an error I was expecting and I'd like to understand why it happens. I could understand the row locking causing deadlock but in this case it seems like the rows are not locked at all. I wonder if the issue is with max() in this context, or possibly the table index. This sproc is the only transaction that is performed on this table.

Any insight is appreciated. I have read several MySql/SO posts on the subject, most concerns and issues seem to be with over-locking or deadlocks. E.g. here: When using MySQL's FOR UPDATE locking, what is exactly locked?

Why not just use an `INTEGER AUTO INCREMENT` or `UUID` for your primary key? Avoids this headache and will make your database a lot faster. — robinsax, Dec 26 '18 at 20:32
@BillKarwin, see if my requirements edit clarifies. I don't think such an index is possible in MySql, if it is, I'd love it if you could show how the table would be defined as an answer. — Aaron Newman, Dec 26 '18 at 21:46
With InnoDB, you must put the auto-increment column as the first column in the PK, and it doesn't number independently for each value in the other column. To do this requires table-locking. E.g. MyISAM can do it. But I don't think this justifies using MyISAM. — Bill Karwin, Dec 31 '18 at 08:02

Rick James · Answer 1 · 2018-12-27T00:54:00.340

To achieve "only the product+key combination needs to be unique", say

UNIQUE(pkKey, productKey)

in either order. Then, your 4-column UNIQUE is redundant. It could be turned into a plain INDEX if needed for some particular query.

Furthermore, you really ought to have a PRIMARY KEY. It may as well be

PRIMARY KEY(pkKey, productKey)  -- in either order

and then get rid of my suggested UNIQUE key.

There is no good reason to make productKey depend on pkKey, if that is what you are thinking of. Instead, simply do

productKey INT UNSIGNED AUTO_INCREMENT

There needs to be at least INDEX(productKey).

Now, I am unclear on whether you need for the 'Menards' and 'Ace' hammers to both be number 121? Summary:

PRIMARY KEY(pkKey, productKey),
INDEX(productKey)

Case 1: Both need to be "121". You need some way to explicitly insert a new row with an existing auto-inc value. This is not a problem; you simply specify '121' instead of letting it acquire the next auto-inc value.

Case 2: There is no need for both to be "121". Then simply use the full force of AUTO_INCREMENT:

PRIMARY KEY(productKey)

But if you really like your SP, let's shorten it down to a single statement, even tossing the transaction:

BEGIN;
    INSERT
         INTO  tblProductKeys 
    SELECT  vkey, vclient, vkeyType,
            @new_id := COALESCE(MAX(productKey) + 1, 0)
        FROM  tblProductKeys
        WHERE  pkKey = vkey
          AND  fkVendor = vvendor
          AND  productType = vkeyType;
END //

Now, you will need

INDEX(pkKey, fkVendor, productType,  -- in any order
      productKey)                    -- last
PRIMARY KEY(pkKey, productKey)  -- in either order (as previously discussed)

Then use @new_id outside the SP.

it is Case 1. Can you explain why your query is atomic and mine is not? It seems another query could get the same max value and increment it if the row is not locked for read. I thought FOR UPDATE accomplished this. — Aaron Newman, Dec 27 '18 at 14:34
I want to thank you for your well-thought-out answer. It was not the issue, but it did inspire me to find the root cause as I will post below. — Aaron Newman, Oct 25 '19 at 16:21

score 0 · Accepted Answer · answered Oct 25 '19 at 16:34

I'm a little embarrassed but it is a pretty obvious problem. The issue is that 'FOR UPDATE' only locks the current row. So you can UPDATE it. But I am doing an INSERT! Not an update.

If 2 queries collide, the row is locked but after the transaction is complete the row is unlocked and it can be read. So you are still reading a stale value. To get the behavior I was expected, you'd need to lock the whole table.

So I think auto-increment would work for me, although I need a way to get the last_inserted_id so I need to be in the context of a procedure anyway (I am using c# driver).

`LAST_INSERT_ID()` is specific to the connection -- _no_ chance of another process messing it up. — Rick James, Oct 26 '19 at 00:22

Select FOR UPDATE gives duplicate key error

2 Answers2