[Dbix-class] DBIx::Class::Ordered changes.

[Peter Rabbitson]

On Wed, Sep 19, 2012 at 06:59:27AM -0700, Bill Moseley wrote:
> On Wed, Sep 19, 2012 at 3:34 AM, Peter Rabbitson <rabbit+dbic at rabbit.us>wrote:
> 
> > > UPDATE ordered_list SET position = position - 1 WHERE ( ( ( position
> > > BETWEEN ? AND ? ) AND owner = ? ) ): '13', '18', '1163299'
> > >
> > > The problem, of course, is I have a UNIQUE( position, owner ) and that
> > last
> > > update isn't guaranteed to work in order (from 13 to18 -- which would
> > > prevent duplicate key errors).
> >
> > The assumption was that databases are able to handle this correctly. Please
> > revert the patch[1] removing this functionality (as per its commit message)
> > and let us know if this solves your problem. Also I'd recommend filing an
> > RT
> > bugreport to keep track of this.
> >
> 
> I've discussed this Ordered code a bit here on the list.   The problem is
> I'm not sure what is the correct fix.  I'm not convinced that the old code
> is better.   I like the idea of doing the move in a single query.
> 
> I can make the UNIQUE( position, group ) constraint DEFERRABLE INITIALLY
> DEFERRED but there's still race conditions.

We can not rely on this being the case. If Pg is not able to do this 
properly as an atomic statement (unlike SQLite and MySQL) then the code 
needs to come back, with a storage flag as discussed... Damn.


> There's room for other updates
> to sneak in between the select and update.   It's the race conditions that
> have seemed to been the most problem in production.    As much as I avoid
> locking, seems like locking is what is needed.

I experimented with some stuff that is still on my disk, trying to set the 
nextpos to (SELECT pos .... ) + 1. It worked for most engines except for Pg 
for which this is not an atomic operation. Then I laughed at Pg and forgot 
about the whole thing. I need to get back to it at some point.

> Years back I added this code for inserts -- where historically we were
> seeing the most duplicate key errors.   After discussion on the Postgresql
> list I ended up doing a select for update *on the related grouping row*.

Except this doesn't work so you do more. You are finding *another* table 
which holds an FK to the grouping column, and use *that* to lock the whole 
thing. Yay for standards compliance :)