[ZODB-Dev] BTree corrupted after conflict resolution

[Tim Peters]

[Toby Dickenson]
>> Sould we break "a == b" equality tests on these stub objects too? I
>> can imagine that being broadly safe today, and I think I remember
>> someone using it.
>>
>> "a is b" better anyway.....

[Dieter Maurer]
> I do not think so.
>
>   *  I do not expect that PersistentReferences to the same
>      peristent object are identical but they should be equal

Nevertheless, it appears that in the current implementation object identity
is preserved by PersistentReference.  Precisely, in the three states passed
to one invocation of conflict resolution, a persisent reference A in one of
them stubs the same oid as a persistent reference B (whether in the same
state or in the other two) if and only if "A is B" holds.  Because the
default __cmp__ is used, a consequence is that "A == B" holds for two
PersistentReferences found in these states today if and only if "A is B"
holds.

>   *  "==" should be defined as "true" when two PersistentReferences
>      reference the same object. This is a safe approximation
>      for object equality
>
>   *  "==" is necessary for non PersistentReferences;
>      prohibiting "==" for PersistentReferences may require
>      weird code to prevent exceptions when the type
>      is not determined before hand. At least, it would complicate
>      things...

I think it's going to painful to change anything about how this works today.
For example, comparing two *values* in a BTree during conflict resolution
uses PyObject_Compare() today.  That code has been wrong for a long time
(forever), because it ignores the possibility that PyObject_Compare() can
raise an exception (someone tried to patch this a year or two ago, but
apparently overlooked this case).  So, if PersistentReference __cmp__ were
changed to raise an exception (but __eq__ and __ne__ were defined to
implement object identity), then:

A. Because the bucket conflict resolution code today has the bug
   described above, it would lead to nasty runtime errors (it's a
   violation of the Python C API to ignore exceptions, and the Python
   virtual machine will eventually go nuts if C code does ignore them).

B. When that bug is fixed, then conflict resolution will start
   raising exceptions even if you just have an IOBTree that maps *to*
   persistent objects (not persistent keys, just persistent values).
   Bucket merge would have to be rewritten to stop calling
   PyObject_Compare() on BTree/Bucket values, and use
   PyObject_RichCompare(..., Py_EQ) instead.

That's all doable (although I don't expect my boss to smile about the time
it would take to do it ...), but because it's subtle I have to wonder how
many users out there are *relying* on all the accidents of how this just
happens to work today.  I'm especially wary because messages here have
claimed that people *do* use persistent objects as BTree keys, despite that
what happens then appears truly and wholly accidental.