saving a named query uses REPLACE INTO, which Sybase doesn't like (and I assume Postgres doesn't either). patch to follow shortly

Comment on attachment 112501 [details] [diff] [review] patch v1 This has a race condidion between the delete and the insert. If I have two transactions, and both try to add an entry which doesn't exist, then both will delete nothing, then try to insert, which will fail the duplicate key tests. You can either: a) LOCK the table first; or b) UPDATE <if no rows updated> LOCK DELETE INSERT UNLOCK <end if> IOW, have a fast-path, and a slow path. I'm not sure if thats worth it here, though, since people don't replace namdqueries that often, and it is a quick operation. Not to mention that the INSERT is a table locking operation anyway for mysql...

one of them was already halfway there, someone just forgot to remove the "REPLACE INTO", which never would have gotten called unless it was an INSERT anyway. Changes the second one to just an INSERT, and adds the if/then to the first one.

Comment on attachment 112579 [details] [diff] [review] patch v2 This is still racy. REPLACE is atomic; UPDATE/INSERT aren't.

The solution is to DELETE (ignore errors) and then INSERT. Preferably inside a transaction.

A more performant way to deal with this is to deal with both error codes you can get: UPDATE IF no rows THEN INSERT IF duplicate key THEN UPDATE This removes the need to do ugly things like lock tables. It's also probably the most database agnostic way to do this, since Oracle and PostgreSQL handle locking comelpetely differently than other databases. Note that the drawback to this is that it doesn't protect against deletes.

New patch that I have also in my PostgreSQL patch. I am trying to break things out into smaller chunks for easier commiting of the PostgreSQL changes later. This patch also adds a couple of other fixes for query.cgi and checksetup.pl. Was a descision ever made whether just performing a LOCK around this section is the way to go or to do a DELETE/INSERT pair?

Comment on attachment 152450 [details] [diff] [review] patch v3 The LOCK alternative would make sense to me.

Added locks to namedqueries table around UPDATE/INSERT operations. I have tested with adding/removing/changing stored queries and seems to work for me. Dave

Comment on attachment 152519 [details] [diff] [review] patch with table locking v4 r=vladd

Looks good to me, too. a= justdave

Checking in buglist.cgi; /cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v <-- buglist.cgi new revision: 1.254; previous revision: 1.253 done Checking in checksetup.pl; /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl new revision: 1.287; previous revision: 1.286 done Checking in query.cgi; /cvsroot/mozilla/webtools/bugzilla/query.cgi,v <-- query.cgi new revision: 1.126; previous revision: 1.125 done