CODEWITHSUNDEEP
javajpaprimary-key
Marcus Junius Brutus
Marcus Junius Brutus

Reputation: 27286

numeric primary key with no sequence used; how to obtain the next value?

I know one is supposed to use a database sequence to obtain the value for the next key on a table (let's assume a single-column numeric Primary Key), but in case a sequence is not used, do you see any obvious code smells with the following code (code is in Java for JPA but the pattern is really language and technology independent):

  boolean haveSucceeded = false;
  int _i = 0 ;
  while ((!haveSucceeded) && (_i++ < MAX_RETRIES)) {
     try {
          user.setId(getFacade().maxId()+1); // line A
          getFacade().create(user);          // line B
          haveSucceeded = true;
     } catch (javax.ejb.EJBTransactionRolledbackException exc) {
          debug("ConstraintValidationException, "+ ( (_i<MAX_RETRIES)?"retrying":"giving up"));
     }
  }

Where the retries are happening to account for possible clashes due to concurrent accesses (possibly from other application instances as well) and since it cannot be guaranteed that line A (where the max is calculated) and line B (where the row is inserted) will operate on the same data.

Upvotes: 0

Views: 133

Answers (2)

Bohemian
Bohemian

Reputation: 424993

This doesn't look thread safe. You should, in a thread-safe way:

  • lock the user table
  • read the current max(id)
  • add 1 and use that as the next id
  • unlock the table

This approximates what happens in the database with an autoincrement.

If you can't lock the table, synchronize the method, either making it static synchronized or using a static lock object, eg:

 private static final Object lock = new Object();

 void yourMethod() {
     synchronized (lock) {
         // your code here
     }
 }

If you have multiple instances of your server running, this approach won't be enough.

Upvotes: 2

nicephotog
nicephotog

Reputation: 11

If you bother to try by retry on access , why not set the "method" when creating the Statement to CONCUR_READ_ONLY or CONCUR_UPDATABLE with interface "Connection".createStatement(). The database itself is what controls the read concurrency for most but the connection can tell the database as part of the query how to read it. Why you want to bother with the loop when there would be more likely an access-problem Exception to occur "if ever" more than any problem with reading concurrency, i don't understand. Niether can i understand why you would want to be so undiscriminate of the data values if you are creating a user, you should check for the users existence first, then abort the process and send a message back to the intending new user "choose another username" e.t.c...

Upvotes: -1

Related Questions