I haven't run into that LOCK TABLE thing before, but some googling suggests that maybe you need to be using raw queryfx() stuff on the same $conn? I'd normally expect the existing w mode $conn to get reused (since we can use a 'w' conn for 'r'), but maybe that isn't actually what happens today.

Using $binding->save() is dangerous anyway because if any future version of Phabricator ever adds a column to AlmanacBinding, the code will stop working. The safe version of this is just to update the table directly, anyway.

So the intermediate query logic should probably just be queryfx(...) stuff for that reason, which will probably fix the LOCK TABLE issue as a side effect?

I'd also probably just directly DELETE the rows being removed for similar reasons -- DestructionEngine has a lot of logging side effects which may break as the codebase moves forward.

I think we'll be left with a semi-weird possible edge case where maybe some stray binding properties get left over if your data was super weird, but we can just live with that.

This change looks right to me structurally.

Okie dokie. I was also wondering if I need to create any transaction objects for reattaching the bindings, but I doubt that will work if I'm restricted to queryfx.

Definitely OK to just make the rows as-correct-as-possible without writing transactions for them.

I think this reattaches the bindings to the interface we delete, not the actual survivor? Maybe I'm crazy but:

• The first interface (say, ID 1) gets marked as $seen.
• The second interface (say, ID 2) actually goes into the logic.
• But we select the SECOND interface (ID 2) as the "$survivor", rebind to it, and then delete it? I think? We should actually be selecting $seen[$key] (ID 1) as the survivor, I think?

This otherwise looks right to me. Not sure if I'm misreading that / missing something. I can try it locally and give you more specific repro instructions if that sounds crazy.

Whoops, good catch, you're right. I copy/pasted incorrectly when migrating to queryfx.