fix: do less in CombineRanges (#5)

odex/optimize.py

@@ -1,5 +1,5 @@
 from collections import defaultdict
-from typing import Callable, Sequence, Dict, TYPE_CHECKING, List, Union as UnionType
+from typing import Callable, Sequence, Dict, TYPE_CHECKING, List
 from typing_extensions import Protocol
 
 from odex.condition import and_, BinOp, Attribute
@@ -10,10 +10,7 @@
     Intersect,
     Filter,
     ScanFilter,
-    Range,
     IndexRange,
-    IndexLookup,
-    Bound,
     Empty,
 )
 
@@ -92,13 +89,10 @@ class CombineRanges(TransformerRule):
 
     def transform(self, plan: Plan, ctx: Context) -> Plan:
         if isinstance(plan, Intersect):
-            # Group the plans by ones that support ranges and by index
-            plans_by_index: Dict[Index, List[UnionType[IndexLookup, IndexRange]]] = defaultdict(
-                list
-            )
+            plans_by_index: Dict[Index, List[IndexRange]] = defaultdict(list)
             others = []
             for i in plan.inputs:
-                if isinstance(i, (IndexLookup, IndexRange)):
+                if isinstance(i, IndexRange):
                     plans_by_index[i.index].append(i)
                 else:
                     others.append(i)
@@ -110,36 +104,22 @@ def transform(self, plan: Plan, ctx: Context) -> Plan:
                     inputs.append(plans[0])
                     continue
 
-                ranges = [
-                    # Treat a lookup as a range
-                    Range(
-                        left=Bound(i.value, True),
-                        right=Bound(i.value, True),
-                    )
-                    if isinstance(i, IndexLookup)
-                    else i.range
-                    for i in plans
-                ]
-
-                new_range = ranges[0]
-                for rng in ranges[1:]:
-                    combined = new_range.combine(rng)
+                new_range = plans[0].range
+                for p in plans[1:]:
+                    combined = new_range.combine(p.range)
 
                     # None means there is a range that always evaluates to False
                     if combined is None:
                         return Empty()
                     else:
                         new_range = combined
 
-                if new_range.is_equality():
-                    assert isinstance(new_range.left, Bound)
-                    new_plan: Plan = IndexLookup(index=index, value=new_range.left.value)
-                else:
-                    new_plan = IndexRange(
+                inputs.append(
+                    IndexRange(
                         index=index,
                         range=new_range,
                     )
-                inputs.append(new_plan)
+                )
 
             inputs.extend(others)
 

odex/plan.py

@@ -181,16 +181,6 @@ def combine(self, other: "Range[C]") -> "Optional[Range[C]]":
             right=right,
         )
 
-    def is_equality(self) -> bool:
-        """Determine if a range can be treated like an equality"""
-        return (
-            isinstance(self.left, Bound)
-            and isinstance(self.right, Bound)
-            and self.left.inclusive
-            and self.right.inclusive
-            and self.left == self.right
-        )
-
     def _combine_bounds(
         self,
         a: OptionalBound,

tests/fixtures/e2e.yaml

@@ -129,7 +129,7 @@ setups:
           - ScanFilter: a >= 3
         - ScanFilter: a <= 3
     optimized_plan: |-
-      IndexLookup: SortedDictIndex(a) = 3
+      IndexRange: 3 <= SortedDictIndex(a) <= 3
     result:
     - 2
   - title: Conflicting equalities
@@ -139,7 +139,9 @@ setups:
         - ScanFilter: a = 1
         - ScanFilter: a = 2
     optimized_plan: |-
-      Empty
+      Intersect
+        - IndexLookup: SortedDictIndex(a) = 1
+        - IndexLookup: SortedDictIndex(a) = 2
     result: []
 - objects:
   - a: 1
@@ -267,15 +269,19 @@ setups:
         - ScanFilter: a = 'foo'
         - ScanFilter: a = 'foo'
     optimized_plan: |-
-      IndexLookup: HashIndex(a) = 'foo'
+      Intersect
+        - IndexLookup: HashIndex(a) = 'foo'
+        - IndexLookup: HashIndex(a) = 'foo'
     result:
-      - 0
+    - 0
   - title: Conflicting equalities, string
     condition: a = 'foo' AND a = 'bar'
     plan: |-
       Intersect
         - ScanFilter: a = 'foo'
         - ScanFilter: a = 'bar'
     optimized_plan: |-
-      Empty
+      Intersect
+        - IndexLookup: HashIndex(a) = 'foo'
+        - IndexLookup: HashIndex(a) = 'bar'
     result: []