views:

858

answers:

5

I've just found the following code:

select max(id) from TABLE_NAME ...

... do some stuff ...

insert into TABLE_NAME (id, ... )
VALUES (max(id) + 1, ...)

I can create a sequence for the PK, but there's a bunch of existing code (classic asp, existing asp.net apps that aren't part of this project) that's not going to use it.

Should I just ignore it, or is there a way to fix it without going into the existing code?

I'm thinking that the best option is just to do:

insert into TABLE_NAME (id, ... )
VALUES (select max(id) + 1, ...)

Options?

+1  A: 

You can create a trigger on the table that overwrites the value for ID with a value that you fetch from a sequence. That way you can still use the other existing code and have no problems with concurrent inserts.

If you cannot change the other software and they still do the select max(id)+1 insert that is most unfortunate. What you then can do is:

For your own insert use a sequence and populate the ID field with -1*(sequence value). This way the insert will not interfere with the existing programs, but also not conflict with the existing programs. (of do the insert without a value for id and use a trigger to populate the ID with the negative value of a sequence).

Edwin
A: 

Big question: does anybody rely on the value of the PK? If not I would recommend using a trigger, fetching the id from a sequence and setting it. The inserts wouldn't specify and id at all.

I am not sure but the

insert into TABLE_NAME (id, ... ) VALUES (select max(id) + 1, ...)

might cause problems when to sessions reach that code. It might be that oracle reads the table (calculating max(id)) and then trys to get the lock on the PK for insertion. If that is the case two concurrent session might try to use the same id, causing an exception in the second session.

You could add some logging to the trigger, to check if inserts get processed that already have an ID set. So you know you have still to hunt down some place where the old code is used.

Jens Schauder
+1  A: 

As others have said, you can override the max value in a database trigger using a sequence. However, that could cause problems if any of the application code uses that value like this:

select max(id) from TABLE_NAME ...

... do some stuff ...

insert into TABLE_NAME (id, ... )
VALUES (max(id) + 1, ...)

insert into CHILD_TABLE (parent_id, ...)
VALUES (max(id) + 1, ...)
Tony Andrews
Unfortunately, that's what appears to be happening - the first select max(id) returns the "new" ID and passes that around, so using a trigger to over-ride the insert won't work.
chris
+1  A: 

Use a seqeunce in a before insert row trigger. select max(id) + 1 doesn't work in a multi concerrency environment.

tuinstoel
You could try LOCK TABLE table_name IN EXCLUSIVE MODE before the select. It will be less concurrent but won't break.
Gary
+1  A: 

This quickly turns in to a discussion of application architecture, especially when the question boils down to "what should I do?"

Primary keys in Oracle really need to come from sequences and since you're dealing with complex insert logic (parent/child inserts, at least) in your application code, you should go into the existing code, as you say (since triggers probably won't help you).

On one extreme you could take away direct SQL access from applications and make them call services so the insert/update/delete code can be centralized. Or you could rewrite your code using some sort of MVC architecture. I'm assuming both are overkill for your situation.

Is the id column at least set to be a true primary key so there's a constraint that will keep duplicates from occurring? If not, start there.

Once the primary key is in place, or if it already is, it's only a matter of time until inserts start to fail; you'll know when they start to fail, right? If not, get on the error-logging.

Now fix the application code. While you're in there, you should at least write and call helper code so your database interactions are in as few places as possible. Then provide some leadership to the other developers and make sure they use the helper code too.

Alkini