Merge pull request #54 from zopefoundation/check-obj-cmp-on-insert-only

Check for object key default comparison on set only

BTrees/BTreeTemplate.c

@@ -726,6 +726,10 @@ _BTree_set(BTree *self, PyObject *keyarg, PyObject *value,
     COPY_KEY_FROM_ARG(key, keyarg, copied);
     if (!copied)
         return -1;
+#ifdef KEY_CHECK_ON_SET
+    if (value && !KEY_CHECK_ON_SET(keyarg))
+        return -1;
+#endif
 
     PER_USE_OR_RETURN(self, -1);
 

BTrees/BucketTemplate.c

@@ -323,6 +323,10 @@ _bucket_set(Bucket *self, PyObject *keyarg, PyObject *v,
     COPY_KEY_FROM_ARG(key, keyarg, copied);
     UNLESS(copied)
         return -1;
+#ifdef KEY_CHECK_ON_SET
+    if (v && !KEY_CHECK_ON_SET(keyarg))
+        return -1;
+#endif
 
     /* Copy the value early (if needed), so that in case of error a
      * pile of bucket mutations don't need to be undone.
@@ -451,7 +455,7 @@ Done:
 /*
 ** bucket_setitem
 **
-** wrapper for _bucket_setitem (eliminates +1 return code)
+** wrapper for _bucket_set (eliminates +1 return code)
 **
 ** Arguments:    self    The bucket
 **        key    The key to insert under

BTrees/Development.txt

@@ -58,6 +58,10 @@ Macros for Keys
     non-zero for true).  When it returns false, its caller should probably set
     a ``TypeError`` exception.
 
+``KEY_CHECK_ON_SET(K)``
+
+    Like ``KEY_CHECK``, but only checked during ``__setitem__``.
+
 ``TEST_KEY_SET_OR(V, K, T)``
 
     Like Python's ``cmp()``.  Compares K(ey) to T(arget), where ``K``

BTrees/objectkeymacros.h

@@ -37,4 +37,5 @@ if ( ( (V) = COMPARE((KEY),(TARGET)) ), PyErr_Occurred() )
 #define COPY_KEY_TO_OBJECT(O, K) O=(K); Py_INCREF(O)
 #define COPY_KEY_FROM_ARG(TARGET, ARG, S) \
     TARGET=(ARG); \
-    (S) = check_argument_cmp(ARG); 
+    (S) = 1;
+#define KEY_CHECK_ON_SET check_argument_cmp

BTrees/tests/test_OOBTree.py

@@ -13,6 +13,7 @@
 ##############################################################################
 import unittest
 
+from .common import _skip_under_Py3k
 from .common import BTreeTests
 from .common import ExtendedSetTests
 from .common import InternalKeysMappingTest
@@ -160,6 +161,21 @@ class OOBTreeTest(BTreeTests, unittest.TestCase):
         self.assertRaises(KeyError, t.__getitem__, C())
         self.assertFalse(C() in t)
 
+    # Check that a None key can be deleted in Python 2.
+    # This doesn't work on Python 3 because None is unorderable,
+    # so the tree can't be searched. But None also can't be inserted,
+    # and we don't support migrating Python 2 databases to Python 3.
+    @_skip_under_Py3k
+    def testDeleteNoneKey(self):
+        t = self._makeOne()
+        bucket_state = ((None, 42),)
+        tree_state = ((bucket_state,),)
+        t.__setstate__(tree_state)
+
+        self.assertEqual(t[None], 42)
+        del t[None]
+
+
 class OOBTreePyTest(OOBTreeTest):
 #
 # Right now, we can't match the C extension's test / prohibition of the

CHANGES.rst

@@ -4,6 +4,13 @@
 4.3.2 (unreleased)
 ------------------
 
+- Make the CPython implementation consistent with the pure-Python
+  implementation and only check object keys for default comparison
+  when setting keys. In Python 2 this makes it possible to remove
+  keys that were added using a less restrictive version of BTrees.
+  (In Python 3 keys that are unorderable still cannot be removed.)
+  See: https://github.com/zopefoundation/BTrees/issues/53
+
 - Make the CPython implementation consistent with the pure-Python
   implementation and no longer raise ``TypeError`` for an object key
   (in object-keyed trees) with default comparison on ``__getitem__``,

tox.ini

@@ -3,7 +3,7 @@ envlist =
 # Jython support pending 2.7 support, due 2012-07-15 or so.  See:
 # http://fwierzbicki.blogspot.com/2012/03/adconion-to-fund-jython-27.html
 #   py27,jython,pypy,coverage,docs
-    py27,py27-pure,pypy,py33,py34,py35,pypy3,w_zodb,coverage,docs
+    py27,py27-pure,pypy,py33,py34,py35,py35-pure,pypy3,w_zodb,coverage,docs
 
 [testenv]
 deps =
@@ -24,6 +24,17 @@ deps =
 commands =
     python setup.py -q test -q
 
+[testenv:py35-pure]
+basepython =
+    python3.5
+setenv =
+    PURE_PYTHON = 1
+    PIP_CACHE_DIR = {envdir}/.cache
+deps =
+    {[testenv]deps}
+commands =
+    python setup.py -q test -q
+
 #[testenv:jython]
 #commands =
 #   jython setup.py test -q