Allow string comparison

This library currently only supports "comparison" of numberic values. This appears to match the JMESPath spec but is not consistent with other implementations (for example: [`jmespath.py`](jmespath/jmespath.py#126)). Resolves: jmespath#47
telus-agcg-archive · Apr 9, 2020 · 9c0cce6 · 9c0cce6
1 parent 1844ca4
commit 9c0cce6
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 8 deletions.
diff --git a/lib/jmespath/nodes/comparator.rb b/lib/jmespath/nodes/comparator.rb
@@ -2,6 +2,8 @@ module JMESPath
   # @api private
   module Nodes
     class Comparator < Node
+      COMPARABLE_TYPES = [Numeric, String].freeze
+
       attr_reader :left, :right
 
       def initialize(left, right)
@@ -36,6 +38,12 @@ def optimize
       def check(left_value, right_value)
         nil
       end
+
+      def comparable?(left_value, right_value)
+        COMPARABLE_TYPES.any? do |type|
+          left_value.is_a?(type) && right_value.is_a?(type)
+        end
+      end
     end
 
     module Comparators
@@ -54,7 +62,7 @@ def check(left_value, right_value)
 
       class Gt < Comparator
         def check(left_value, right_value)
-          if left_value.is_a?(Numeric) && right_value.is_a?(Numeric)
+          if comparable?(left_value, right_value)
             left_value > right_value
           else
             nil
@@ -64,7 +72,8 @@ def check(left_value, right_value)
 
       class Gte < Comparator
         def check(left_value, right_value)
-          if left_value.is_a?(Numeric) && right_value.is_a?(Numeric)
+          # raise "#{left_value} <-> #{right_value}"
+          if comparable?(left_value, right_value)
             left_value >= right_value
           else
             nil
@@ -74,7 +83,7 @@ def check(left_value, right_value)
 
       class Lt < Comparator
         def check(left_value, right_value)
-          if left_value.is_a?(Numeric) && right_value.is_a?(Numeric)
+          if comparable?(left_value, right_value)
             left_value < right_value
           else
             nil
@@ -84,7 +93,7 @@ def check(left_value, right_value)
 
       class Lte < Comparator
         def check(left_value, right_value)
-          if left_value.is_a?(Numeric) && right_value.is_a?(Numeric)
+          if comparable?(left_value, right_value)
             left_value <= right_value
           else
             nil

diff --git a/lib/jmespath/nodes/condition.rb b/lib/jmespath/nodes/condition.rb
@@ -27,6 +27,7 @@ def optimize
 
     class ComparatorCondition < Node
       COMPARATOR_TO_CONDITION = {}
+      COMPARABLE_TYPES = [Integer, String].freeze
 
       def initialize(left, right, child)
         @left = left
@@ -37,6 +38,14 @@ def initialize(left, right, child)
       def visit(value)
         nil
       end
+
+      private
+
+      def comparable?(left_value, right_value)
+        COMPARABLE_TYPES.any? do |type|
+          left_value.is_a?(type) && right_value.is_a?(type)
+        end
+      end
     end
 
     class EqCondition < ComparatorCondition
@@ -99,7 +108,7 @@ class GtCondition < ComparatorCondition
       def visit(value)
         left_value = @left.visit(value)
         right_value = @right.visit(value)
-        left_value.is_a?(Integer) && right_value.is_a?(Integer) && left_value > right_value ? @child.visit(value) : nil
+        comparable?(left_value, right_value) && left_value > right_value ? @child.visit(value) : nil
       end
     end
 
@@ -109,7 +118,7 @@ class GteCondition < ComparatorCondition
       def visit(value)
         left_value = @left.visit(value)
         right_value = @right.visit(value)
-        left_value.is_a?(Integer) && right_value.is_a?(Integer) && left_value >= right_value ? @child.visit(value) : nil
+        comparable?(left_value, right_value) && left_value >= right_value ? @child.visit(value) : nil
       end
     end
 
@@ -119,7 +128,7 @@ class LtCondition < ComparatorCondition
       def visit(value)
         left_value = @left.visit(value)
         right_value = @right.visit(value)
-        left_value.is_a?(Integer) && right_value.is_a?(Integer) && left_value < right_value ? @child.visit(value) : nil
+        comparable?(left_value, right_value) && left_value < right_value ? @child.visit(value) : nil
       end
     end
 
@@ -129,7 +138,7 @@ class LteCondition < ComparatorCondition
       def visit(value)
         left_value = @left.visit(value)
         right_value = @right.visit(value)
-        left_value.is_a?(Integer) && right_value.is_a?(Integer) && left_value <= right_value ? @child.visit(value) : nil
+        comparable?(left_value, right_value) && left_value <= right_value ? @child.visit(value) : nil
       end
     end
   end

diff --git a/spec/compliance/boolean.json b/spec/compliance/boolean.json
@@ -269,6 +269,14 @@
       {
         "expression": "two < one || three < one",
         "result": false
+      },
+      {
+        "expression": "'2010-02-01' > '2011-05-01'",
+        "result": false
+      },
+      {
+        "expression": "'2010-02-01' <= '2011-05-01'",
+        "result": true
       }
     ]
   }

diff --git a/spec/compliance/filters.json b/spec/compliance/filters.json
@@ -464,5 +464,29 @@
         "result": [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
       }
     ]
+  },
+  {
+    "given": {
+      "foo": ["2010-02-01", "2011-05-01"]
+    },
+    "cases": [
+      {
+        "comment": "Greater than with ISO 8601 date string",
+        "expression": "foo[?@ > '2010-06-01']",
+        "result": ["2011-05-01"]
+      }
+    ]
+  },
+  {
+    "given": {
+      "foo": [{"date": "2010-02-01"}, {"date": "2011-05-01"}]
+    },
+    "cases": [
+      {
+        "comment": "Greater than with ISO 8601 date string",
+        "expression": "foo[?date > '2010-06-01'].date",
+        "result": ["2011-05-01"]
+      }
+    ]
   }
 ]