Fix catalog plan for not query (#139)

* Fix catalog plan for not query

* fix

CHANGES.rst

@@ -7,6 +7,9 @@ Changelog
 - Improve performance stability. Fix catalog plan for unused index in a query.
   (`#138 <https://github.com/zopefoundation/Products.ZCatalog/pull/138>`_)
 
+- Improve performance stability. Fix catalog plan for not query.
+  (`#139 <https://github.com/zopefoundation/Products.ZCatalog/pull/139>`_)
+
 
 6.2 (2022-04-08)
 ----------------

src/Products/ZCatalog/plan.py

@@ -244,7 +244,13 @@ def make_key(self, query):
                 # repr() is an easy way to do this without imposing
                 # restrictions on the types of values.
                 key.append((name, repr(v)))
-
+        notkeys = [
+            name for name in key
+            if isinstance(query.get(name), dict) and "not" in query[name]
+        ]
+        if notkeys:
+            key = [name for name in key if name not in notkeys]
+            key.extend([(name, "not") for name in notkeys])
         # Workaround: Python 2.x accepted different types as sort key
         # for the sorted builtin. Python 3 only sorts on identical types.
         tuple_keys = set(key) - set(

src/Products/ZCatalog/tests/test_plan.py

@@ -313,6 +313,49 @@ def query_index(self, record, resultset=None):
             cat.getCatalogPlan(query2).plan(), ["numbers", "num", "date"]
         )
 
+    def test_not_query(self):
+        # not query is generally slower, force this behavior for testing
+        class SlowNotFieldIndex(FieldIndex):
+            def query_index(self, record, resultset=None):
+                if getattr(record, 'not', None):
+                    time.sleep(0.1)
+                return super(SlowNotFieldIndex, self).query_index(
+                    record, resultset)
+
+        zcat = ZCatalog("catalog")
+        cat = zcat._catalog
+        cat.addIndex(
+            'num1', SlowNotFieldIndex('num1', extra={"indexed_attrs": "num"}))
+        cat.addIndex(
+            'num2', SlowNotFieldIndex('num2', extra={"indexed_attrs": "num"}))
+        for i in range(100):
+            obj = Dummy(i)
+            zcat.catalog_object(obj, str(i))
+
+        query1 = {"num1": {"not": 2}, "num2": 3}
+        query2 = {"num1": 2, "num2": {'not': 5}}
+
+        # without a plan index are orderd alphabetically by default
+        for query in [query1, query2]:
+            self.assertEqual(zcat._catalog.getCatalogPlan(query).plan(), None)
+            self.assertEqual(
+                cat._sorted_search_indexes(query),
+                ["num1", "num2"]
+            )
+
+        self.assertEqual([b.getPath() for b in zcat.search(query1)], ['3'])
+        self.assertEqual([b.getPath() for b in zcat.search(query2)], ['2'])
+        # although there are the same fields, the plans are different, and the
+        # slower `not` query put the field as second in the plan
+        self.assertEqual(cat.getCatalogPlan(query1).plan(), ["num2", "num1"])
+        self.assertEqual(cat.getCatalogPlan(query2).plan(), ["num1", "num2"])
+
+        # search again doesn't change the order
+        self.assertEqual([b.getPath() for b in zcat.search(query1)], ['3'])
+        self.assertEqual([b.getPath() for b in zcat.search(query2)], ['2'])
+        self.assertEqual(cat.getCatalogPlan(query1).plan(), ["num2", "num1"])
+        self.assertEqual(cat.getCatalogPlan(query2).plan(), ["num1", "num2"])
+
     def test_plan_empty(self):
         plan = self._makeOne()
         self.assertEqual(plan.plan(), None)