Check to Make Sure My New Record Doesn’t Already Exist

Here’s another interesting piece of SQL that I ran into last week:

select con_id from xyz_blah where con_id=’BS-002342′;

Actually there was a whole set of them with different literals. My first thought was, “Why would the developers want to select the con_id when they already had the con_id?”, quickly followed by “Ohhhhhhhh, it’s one of those deals where the developers didn’t trust the database or don’t know how to check for an error after executing a SQL statement”.

Presumably they know that ‘BS-002342′ is a valid con_id (it looks pretty specific to me). So the app’s probably doing one of the following three bad things (listed in what I think is the most likely order).

  1. Checking to make sure a record exists with that con_id before doing something (UPDATE or DELETE).
  2. Checking to make sure that a record doesn’t already exist with that con_id, before doing an INSERT.
  3. Checking to make sure a DELETE actually worked.

When I looked at the stats in v$sql, the queries never return any rows. So it’s probably not #1. I didn’t find any DELETEs on the table but I did find INSERT statements with matching con_id’s, so it looks like it’s the check before insert scenario (#2). There is a Primary Key on the con_id field, so the check is of course, totally unnecessary. They should have just done the insert and handled the duplicate key error if one ever happened. By the way, in the 30 days of AWR data we had available, none of these statements ever returned a row. So most likely, they never have this issue in the first place. Of course, they might also consider using a sequence to generate the key instead of having the app manufacture a 9 character key!

Anyway, this is one of a whole set of coding issues where unnecessary work is done as a standard coding practice. Like Cary Millsap always says, “the fastest way to do anything is not to do it at all”.

2 Comments

  1. HF says:

    I’ve worked on a couple of apps where the app has had no graceful way to handle a sql-error. Usually these apps are aimed at non-technical users who don’t want to see a tech message. So to avoid it the developer does the above. With the use of the literal I wonder if it has been accepted from a front end gui? I also wonder if you could try a ‘; DROP TABLE XYZ_BLAH;’ as the data entry value….

  2. osborne says:

    Actually this is a straight java app without anything like hibernate between the app and the database. So they should be able to tell if a statement succeeded or not. I wouldn’t be surprised if they were vulnerable to SQL injection though as they do a lot of dynamic SQL.

    Kerry

Leave a Reply