From 4150ce6482730999b8909fa2af9ca03905e34047 Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Mon, 15 Apr 2024 15:10:48 -0400
Subject: [PATCH 1/8] miq_expression: convert_size_in_units_to_integer(field,
 column_details, value)

basically just moving up the type checking to the method caller
This changes the method interface so we can pass in a field object instead of calling col_details
---
 lib/miq_expression.rb | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/lib/miq_expression.rb b/lib/miq_expression.rb
index 4f74c199ebf..7de32bbe0a7 100644
--- a/lib/miq_expression.rb
+++ b/lib/miq_expression.rb
@@ -344,7 +344,19 @@ def preprocess_exp!(exp)
       preprocess_exp!(operator_values)
       exp
     else # field
-      convert_size_in_units_to_integer(exp) if %w[= != <= >= > <].include?(operator)
+      # op => {"regkey"=>"foo", "regval"=>"bar", "value"=>"baz"}
+      # op => {"field" => "foo", "value" => "baz"}
+      # op => {"field" => "<count>, "value" => "0"}
+      # op => {"count" => "Vm.snapshots", "value"=>"1"}
+      # op => {"tag"=>"Host.managed-environment", "value"=>"prod"}
+      field = operator_values["field"]
+      column_details = col_details[field] if field
+      value = operator_values["value"]
+
+      # attempt to do conversion only if db type of column is integer and value to compare to is String
+      if %w[= != <= >= > <].include?(operator) && field && column_details && column_details[:data_type] == :integer && value.instance_of?(String)
+        operator_values["value"] = convert_size_in_units_to_integer(field, column_details, value)
+      end
     end
     exp
   end
@@ -1394,24 +1406,19 @@ def fields(expression = exp)
 
   private
 
-  def convert_size_in_units_to_integer(exp)
-    return if (column_details = col_details[exp.values.first["field"]]).nil?
-    # attempt to do conversion only if db type of column is integer and value to compare to is String
-    return unless column_details[:data_type] == :integer && (value = exp.values.first["value"]).instance_of?(String)
-
-    sub_type = column_details[:format_sub_type]
-
-    return if %i[mhz_avg hours kbps kbps_precision_2 mhz elapsed_time].include?(sub_type)
-
-    case sub_type
+  def convert_size_in_units_to_integer(field, column_details, value)
+    case column_details[:format_sub_type]
+    when :mhz_avg, :hours, :kbps, :kbps_precision_2, :mhz, :elapsed_time, :integer
+      value
     when :bytes
-      exp.values.first["value"] = value.to_i_with_method
+      value.to_i_with_method
     when :kilobytes
-      exp.values.first["value"] = value.to_i_with_method / 1_024
+      value.to_i_with_method / 1_024
     when :megabytes, :megabytes_precision_2
-      exp.values.first["value"] = value.to_i_with_method / 1_048_576
+      value.to_i_with_method / 1_048_576
     else
-      _log.warn("No subtype defined for column #{exp.values.first["field"]} in 'miq_report_formats.yml'")
+      _log.warn("No subtype defined for column #{field} in 'miq_report_formats.yml'")
+      value
     end
   end
 

From 87ec5997077306257f0488f1cd0826ac0f6d37f5 Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Mon, 19 Feb 2024 21:41:14 -0500
Subject: [PATCH 2/8] miq_expression preprocess_exp! => preprocess_exp

---
 lib/miq_expression.rb           | 15 +++++++--------
 spec/lib/miq_expression_spec.rb | 12 ++++++------
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/lib/miq_expression.rb b/lib/miq_expression.rb
index 7de32bbe0a7..80ed1e22773 100644
--- a/lib/miq_expression.rb
+++ b/lib/miq_expression.rb
@@ -181,7 +181,7 @@ def to_ruby(timezone = nil, prune_sql: false)
     elsif @ruby
       @ruby.dup
     elsif valid?
-      pexp = preprocess_exp!(exp.deep_clone)
+      pexp = preprocess_exp(exp)
       pexp, _ = prune_exp(pexp, MODE_RUBY) if prune_sql
       @ruby = self.class._to_ruby(pexp, context_type, timezone) || true
       @ruby == true ? nil : @ruby.dup
@@ -325,7 +325,7 @@ def self._to_ruby(exp, context_type, tz)
 
   def to_sql(tz = nil)
     tz ||= "UTC"
-    pexp = preprocess_exp!(exp.deep_clone)
+    pexp = preprocess_exp(exp)
     pexp, seen = prune_exp(pexp, MODE_SQL)
     attrs = {:supported_by_sql => (seen == MODE_SQL)}
     sql = to_arel(pexp, tz).to_sql if pexp.present?
@@ -333,22 +333,21 @@ def to_sql(tz = nil)
     [sql, incl, attrs]
   end
 
-  def preprocess_exp!(exp)
-    exp.delete(:token)
+  def preprocess_exp(exp)
     operator = exp.keys.first
     operator_values = exp[operator]
     case operator.downcase
     when "and", "or"
-      operator_values.each { |atom| preprocess_exp!(atom) }
+      operator_values = operator_values.map { |atom| preprocess_exp(atom) }
     when "not", "!"
-      preprocess_exp!(operator_values)
-      exp
+      operator_values = preprocess_exp(operator_values)
     else # field
       # op => {"regkey"=>"foo", "regval"=>"bar", "value"=>"baz"}
       # op => {"field" => "foo", "value" => "baz"}
       # op => {"field" => "<count>, "value" => "0"}
       # op => {"count" => "Vm.snapshots", "value"=>"1"}
       # op => {"tag"=>"Host.managed-environment", "value"=>"prod"}
+      operator_values = operator_values.dup
       field = operator_values["field"]
       column_details = col_details[field] if field
       value = operator_values["value"]
@@ -358,7 +357,7 @@ def preprocess_exp!(exp)
         operator_values["value"] = convert_size_in_units_to_integer(field, column_details, value)
       end
     end
-    exp
+    {operator => operator_values}
   end
 
   # @param operator [String]      operator (i.e.: AND, OR, NOT)
diff --git a/spec/lib/miq_expression_spec.rb b/spec/lib/miq_expression_spec.rb
index 4ce0f2af171..ea5843826c1 100644
--- a/spec/lib/miq_expression_spec.rb
+++ b/spec/lib/miq_expression_spec.rb
@@ -222,21 +222,21 @@
     end
   end
 
-  describe "#preprocess_exp!" do
+  describe "#preprocess_exp" do
     it "convert size value in units to integer for comparasing operators on integer field" do
       expession_hash = {"=" => {"field" => "Vm-allocated_disk_storage", "value" => "5.megabytes"}}
       expession = MiqExpression.new(expession_hash)
-      exp, _ = expession.preprocess_exp!(expession_hash)
+      exp, _ = expession.preprocess_exp(expession_hash)
       expect(exp.values.first["value"]).to eq("5.megabyte".to_i_with_method)
 
       expession_hash = {">" => {"field" => "Vm-allocated_disk_storage", "value" => "5.kilobytes"}}
       expession = MiqExpression.new(expession_hash)
-      exp, _ = expession.preprocess_exp!(expession_hash)
+      exp, _ = expession.preprocess_exp(expession_hash)
       expect(exp.values.first["value"]).to eq("5.kilobytes".to_i_with_method)
 
       expession_hash = {"<" => {"field" => "Vm-allocated_disk_storage", "value" => "2.terabytes"}}
       expession = MiqExpression.new(expession_hash)
-      exp, _ = expession.preprocess_exp!(expession_hash)
+      exp, _ = expession.preprocess_exp(expession_hash)
       expect(exp.values.first["value"]).to eq(2.terabytes.to_i_with_method)
     end
   end
@@ -3494,13 +3494,13 @@
 
   def sql_pruned_exp(input)
     mexp = MiqExpression.new(input)
-    pexp = mexp.preprocess_exp!(mexp.exp.deep_clone)
+    pexp = mexp.preprocess_exp(mexp.exp)
     mexp.prune_exp(pexp, MiqExpression::MODE_SQL).first
   end
 
   def ruby_pruned_exp(input)
     mexp = MiqExpression.new(input)
-    pexp = mexp.preprocess_exp!(mexp.exp.deep_clone)
+    pexp = mexp.preprocess_exp(mexp.exp)
     mexp.prune_exp(pexp, MiqExpression::MODE_RUBY).first
   end
 end

From dd285472c28df1508bfba76aa77a39789a0b5fe4 Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Thu, 28 Mar 2024 23:40:54 -0400
Subject: [PATCH 3/8] miq_expression embed field object in internal exp

use that value when converting datatype
---
 lib/miq_expression.rb           | 10 +++++-----
 lib/miq_expression/target.rb    |  4 ++++
 spec/lib/miq_expression_spec.rb | 32 +++++++++++++++++---------------
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/lib/miq_expression.rb b/lib/miq_expression.rb
index 80ed1e22773..7141662289f 100644
--- a/lib/miq_expression.rb
+++ b/lib/miq_expression.rb
@@ -349,12 +349,12 @@ def preprocess_exp(exp)
       # op => {"tag"=>"Host.managed-environment", "value"=>"prod"}
       operator_values = operator_values.dup
       field = operator_values["field"]
-      column_details = col_details[field] if field
+      field_field = operator_values["field-field"] = Field.parse(field) if field
       value = operator_values["value"]
 
       # attempt to do conversion only if db type of column is integer and value to compare to is String
-      if %w[= != <= >= > <].include?(operator) && field && column_details && column_details[:data_type] == :integer && value.instance_of?(String)
-        operator_values["value"] = convert_size_in_units_to_integer(field, column_details, value)
+      if %w[= != <= >= > <].include?(operator) && field_field&.integer? && value.instance_of?(String)
+        operator_values["value"] = convert_size_in_units_to_integer(field, field_field.sub_type, value)
       end
     end
     {operator => operator_values}
@@ -1405,8 +1405,8 @@ def fields(expression = exp)
 
   private
 
-  def convert_size_in_units_to_integer(field, column_details, value)
-    case column_details[:format_sub_type]
+  def convert_size_in_units_to_integer(field, sub_type, value)
+    case sub_type
     when :mhz_avg, :hours, :kbps, :kbps_precision_2, :mhz, :elapsed_time, :integer
       value
     when :bytes
diff --git a/lib/miq_expression/target.rb b/lib/miq_expression/target.rb
index c8f058599fd..5f929d59191 100644
--- a/lib/miq_expression/target.rb
+++ b/lib/miq_expression/target.rb
@@ -63,6 +63,10 @@ def decimal?
     column_type == :decimal
   end
 
+  def integer?
+    column_type == :integer
+  end
+
   def tag?
     false
   end
diff --git a/spec/lib/miq_expression_spec.rb b/spec/lib/miq_expression_spec.rb
index ea5843826c1..887cd3e0189 100644
--- a/spec/lib/miq_expression_spec.rb
+++ b/spec/lib/miq_expression_spec.rb
@@ -242,12 +242,14 @@
   end
 
   describe "#reduce_exp" do
-    let(:sql_field)  { {"=" => {"field" => "Vm-name", "value" => "foo"}.freeze}.freeze }
-    let(:ruby_field) { {"=" => {"field" => "Vm-platform", "value" => "bar"}.freeze}.freeze }
+    let(:sql_field)      { {"=" => {"field" => "Vm-name", "value" => "foo"}.freeze}.freeze }
+    let(:sql_field_out)  { {"=" => {"field" => "Vm-name", "field-field" => MiqExpression::Field.parse("Vm-name"), "value" => "foo"}.freeze}.freeze }
+    let(:ruby_field)     { {"=" => {"field" => "Vm-platform", "value" => "bar"}.freeze}.freeze }
+    let(:ruby_field_out) { {"=" => {"field" => "Vm-platform", "field-field" => MiqExpression::Field.parse("Vm-platform"), "value" => "bar"}.freeze}.freeze }
 
     context "mode: :sql" do
       it "(sql AND ruby) => (sql)" do
-        expect(sql_pruned_exp("AND" => [sql_field, ruby_field.clone])).to eq(sql_field)
+        expect(sql_pruned_exp("AND" => [sql_field, ruby_field.clone])).to eq(sql_field_out)
       end
 
       it "(ruby AND ruby) => ()" do
@@ -255,7 +257,7 @@
       end
 
       it "(sql OR sql) => (sql OR sql)" do
-        expect(sql_pruned_exp("OR" => [sql_field, sql_field])).to eq("OR" => [sql_field, sql_field])
+        expect(sql_pruned_exp("OR" => [sql_field, sql_field])).to eq("OR" => [sql_field_out, sql_field_out])
       end
 
       it "(sql OR ruby) => ()" do
@@ -267,7 +269,7 @@
       end
 
       it "!(sql OR ruby) => (!(sql) AND !(ruby)) => !(sql)" do
-        expect(sql_pruned_exp("NOT" => {"OR" => [sql_field, ruby_field.clone]})).to eq("NOT" => sql_field)
+        expect(sql_pruned_exp("NOT" => {"OR" => [sql_field, ruby_field.clone]})).to eq("NOT" => sql_field_out)
       end
 
       it "!(sql AND ruby) => (!(sql) OR !(ruby)) => nil" do
@@ -281,7 +283,7 @@
       end
 
       it "(ruby) => (ruby)" do
-        expect(ruby_pruned_exp(ruby_field.clone)).to eq(ruby_field)
+        expect(ruby_pruned_exp(ruby_field.clone)).to eq(ruby_field_out)
       end
 
       it "(sql and sql) => ()" do
@@ -289,11 +291,11 @@
       end
 
       it "(sql and ruby) => (ruby)" do
-        expect(ruby_pruned_exp("AND" => [sql_field.clone, ruby_field.clone])).to eq(ruby_field)
+        expect(ruby_pruned_exp("AND" => [sql_field.clone, ruby_field.clone])).to eq(ruby_field_out)
       end
 
       it "(ruby or ruby) => (ruby or ruby)" do
-        expect(ruby_pruned_exp("OR" => [ruby_field.clone, ruby_field.clone])).to eq("OR" => [ruby_field, ruby_field])
+        expect(ruby_pruned_exp("OR" => [ruby_field.clone, ruby_field.clone])).to eq("OR" => [ruby_field_out, ruby_field_out])
       end
 
       it "(sql or sql) => ()" do
@@ -301,25 +303,25 @@
       end
 
       it "(sql or ruby) => (sql or ruby)" do
-        expect(ruby_pruned_exp("OR" => [ruby_field.clone, sql_field.clone])).to eq("OR" => [ruby_field, sql_field])
+        expect(ruby_pruned_exp("OR" => [ruby_field.clone, sql_field.clone])).to eq("OR" => [ruby_field_out, sql_field_out])
       end
 
       it "(ruby or ruby) => (ruby or ruby)" do
-        expect(ruby_pruned_exp("OR" => [ruby_field.clone, ruby_field.clone])).to eq("OR" => [ruby_field, ruby_field])
+        expect(ruby_pruned_exp("OR" => [ruby_field.clone, ruby_field.clone])).to eq("OR" => [ruby_field_out, ruby_field_out])
       end
 
       it "(sql AND sql) or ruby => keep all expressions" do
-        expect(ruby_pruned_exp("OR" => [{"AND" => [sql_field.clone, sql_field.clone]}, ruby_field.clone])).to eq("OR" => [{"AND" => [sql_field, sql_field]}, ruby_field])
+        expect(ruby_pruned_exp("OR" => [{"AND" => [sql_field.clone, sql_field.clone]}, ruby_field.clone])).to eq("OR" => [{"AND" => [sql_field_out, sql_field_out]}, ruby_field_out])
       end
 
       # ensuring that the OR/AND is treating each sub expression independently
       # it was getting this wrong
       it "(sql or sql) and ruby => ruby" do
-        expect(ruby_pruned_exp("AND" => [{"OR" => [sql_field.clone, sql_field.clone]}, ruby_field.clone])).to eq(ruby_field)
+        expect(ruby_pruned_exp("AND" => [{"OR" => [sql_field.clone, sql_field.clone]}, ruby_field.clone])).to eq(ruby_field_out)
       end
 
       it "ruby and (sql or sql) => ruby" do
-        expect(ruby_pruned_exp("AND" => [ruby_field.clone, {"OR" => [sql_field.clone, sql_field.clone]}])).to eq(ruby_field)
+        expect(ruby_pruned_exp("AND" => [ruby_field.clone, {"OR" => [sql_field.clone, sql_field.clone]}])).to eq(ruby_field_out)
       end
 
       it "!(ruby) => keep all expressions" do
@@ -329,11 +331,11 @@
       end
 
       it "!(sql OR ruby) => (!(sql) AND !(ruby)) => !(ruby)" do
-        expect(ruby_pruned_exp("NOT" => {"OR" => [sql_field, ruby_field.clone]})).to eq("NOT" => ruby_field)
+        expect(ruby_pruned_exp("NOT" => {"OR" => [sql_field, ruby_field.clone]})).to eq("NOT" => ruby_field_out)
       end
 
       it "!(sql AND ruby) => (!(sql) OR !(ruby)) => !(sql AND ruby)" do
-        expect(ruby_pruned_exp("NOT" => {"AND" => [sql_field, ruby_field.clone]})).to eq("NOT" => {"AND" => [sql_field, ruby_field]})
+        expect(ruby_pruned_exp("NOT" => {"AND" => [sql_field, ruby_field.clone]})).to eq("NOT" => {"AND" => [sql_field_out, ruby_field_out]})
       end
     end
   end

From d8ec1f3a708ba853e60c312505063ef28abb58ba Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Thu, 7 Mar 2024 11:44:42 -0500
Subject: [PATCH 4/8] miq_expression leverage more miq_expression field-field
 values

---
 lib/miq_expression.rb           | 24 ++++++------------
 spec/lib/miq_expression_spec.rb | 44 ++++++++++++++++-----------------
 2 files changed, 29 insertions(+), 39 deletions(-)

diff --git a/lib/miq_expression.rb b/lib/miq_expression.rb
index 7141662289f..91176dc9542 100644
--- a/lib/miq_expression.rb
+++ b/lib/miq_expression.rb
@@ -196,6 +196,7 @@ def self._to_ruby(exp, context_type, tz)
     operator = exp.keys.first
     op_args = exp[operator]
     col_name = op_args["field"] if op_args.kind_of?(Hash)
+    field    = op_args["field-field"] if op_args.kind_of?(Hash)
     operator = operator.downcase
 
     case operator
@@ -484,14 +485,14 @@ def sql_supports_atom?(exp)
       if exp[operator].key?("tag")
         Tag.parse(exp[operator]["tag"]).reflection_supported_by_sql?
       elsif exp[operator].key?("field")
-        Field.parse(exp[operator]["field"]).attribute_supported_by_sql?
+        exp[operator]["field-field"].attribute_supported_by_sql?
       else
         false
       end
     when "includes"
       # Support includes operator using "LIKE" only if first operand is in main table
       if exp[operator].key?("field") && (!exp[operator]["field"].include?(".") || (exp[operator]["field"].include?(".") && exp[operator]["field"].split(".").length == 2))
-        field_in_sql?(exp[operator]["field"])
+        field_in_sql?(exp[operator]["field"], exp[operator]["field-field"])
       else
         # TODO: Support includes operator for sub-sub-tables
         false
@@ -525,21 +526,10 @@ def value_in_sql?(value)
     !Field.is_field?(value) || Field.parse(value).attribute_supported_by_sql?
   end
 
-  def field_in_sql?(field)
-    return false unless attribute_supported_by_sql?(field)
-
-    # => false if excluded by special case defined in preprocess options
-    return false if field_excluded_by_preprocess_options?(field)
-
-    true
-  end
-
-  def attribute_supported_by_sql?(field)
-    return false unless col_details[field]
-
-    col_details[field][:sql_support]
+  def field_in_sql?(field, field_field)
+    field_field.attribute_supported_by_sql? &&
+      !field_excluded_by_preprocess_options?(field)
   end
-  # private attribute_supported_by_sql? -- tests only
 
   def field_excluded_by_preprocess_options?(field)
     col_details[field][:excluded_by_preprocess_options]
@@ -1440,7 +1430,7 @@ def self.ruby_for_date_compare(col_ruby, col_type, tz, op1, val1, op2 = nil, val
 
   def to_arel(exp, tz)
     operator = exp.keys.first
-    field = Field.parse(exp[operator]["field"]) if exp[operator].kind_of?(Hash) && exp[operator]["field"]
+    field = exp[operator]["field-field"] if exp[operator].kind_of?(Hash)
     arel_attribute = field&.arel_attribute
     if exp[operator].kind_of?(Hash) && exp[operator]["value"] && Field.is_field?(exp[operator]["value"])
       field_value = Field.parse(exp[operator]["value"])
diff --git a/spec/lib/miq_expression_spec.rb b/spec/lib/miq_expression_spec.rb
index 887cd3e0189..018e36472a4 100644
--- a/spec/lib/miq_expression_spec.rb
+++ b/spec/lib/miq_expression_spec.rb
@@ -3041,25 +3041,25 @@
       context "operation with 'field'" do
         it "returns false if format of field is model.association.association-field" do
           field = "ManageIQ::Providers::InfraManager::Vm.service.user.vms-active"
-          expression = {"CONTAINS" => {"field" => field, "value" => "true"}}
+          expression = {"CONTAINS" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "true"}}
           expect(described_class.new(nil).sql_supports_atom?(expression)).to eq(false)
         end
 
         it "returns false if field belongs to virtual_has_many association" do
           field = "ManageIQ::Providers::InfraManager::Vm.processes-type"
-          expression = {"CONTAINS" => {"field" => field, "value" => "abc"}}
+          expression = {"CONTAINS" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "abc"}}
           expect(described_class.new(nil).sql_supports_atom?(expression)).to eq(false)
         end
 
         it "returns false if field belongs to 'has_many' polymorhic/polymorhic association" do
           field = "ManageIQ::Providers::InfraManager::Vm.advanced_settings-region_number"
-          expression = {"CONTAINS" => {"field" => field, "value" => "1"}}
+          expression = {"CONTAINS" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "1"}}
           expect(described_class.new(nil).sql_supports_atom?(expression)).to eq(false)
         end
 
         it "returns true if field belongs to 'has_many' association" do
           field = "ManageIQ::Providers::InfraManager::Vm.registry_items-name"
-          expression = {"CONTAINS" => {"field" => field, "value" => "abc"}}
+          expression = {"CONTAINS" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "abc"}}
           expect(described_class.new(expression).sql_supports_atom?(expression)).to eq(true)
         end
       end
@@ -3068,31 +3068,31 @@
     context "expression key is 'INCLUDE'" do
       it "returns false for model-virtualfield" do
         field = "ManageIQ::Providers::InfraManager::Vm-v_datastore_path"
-        expression = {"INCLUDES" => {"field" => field, "value" => "abc"}}
+        expression = {"INCLUDES" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "abc"}}
         expect(described_class.new(expression).sql_supports_atom?(expression)).to eq(false)
       end
 
       it "returns true for model-field" do
         field = "ManageIQ::Providers::InfraManager::Vm-location"
-        expression = {"INCLUDES" => {"field" => field, "value" => "abc"}}
+        expression = {"INCLUDES" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "abc"}}
         expect(described_class.new(expression).sql_supports_atom?(expression)).to eq(true)
       end
 
       it "returns false for model.association.virtualfield" do
         field = "ManageIQ::Providers::InfraManager::Vm.ext_management_system-hostname"
-        expression = {"INCLUDES" => {"field" => field, "value" => "abc"}}
+        expression = {"INCLUDES" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "abc"}}
         expect(described_class.new(expression).sql_supports_atom?(expression)).to eq(false)
       end
 
       it "returns true for model.accociation.field" do
         field = "ManageIQ::Providers::InfraManager::Vm.ext_management_system-name"
-        expression = {"INCLUDES" => {"field" => field, "value" => "abc"}}
+        expression = {"INCLUDES" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "abc"}}
         expect(described_class.new(expression).sql_supports_atom?(expression)).to eq(true)
       end
 
       it "returns false if format of field is model.association..association-field" do
         field = "ManageIQ::Providers::InfraManager::Vm.service.miq_request-v_approved_by"
-        expression = {"INCLUDES" => {"field" => field, "value" => "abc"}}
+        expression = {"INCLUDES" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "abc"}}
         expect(described_class.new(expression).sql_supports_atom?(expression)).to eq(false)
       end
     end
@@ -3127,31 +3127,31 @@
 
     it "supports sql for model.association-virtualfield (with arel)" do
       field = "Host.vms-archived"
-      expression = {"=" => {"field" => field, "value" => "true"}}
+      expression = {"=" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "true"}}
       expect(described_class.new(expression).sql_supports_atom?(expression)).to eq(true)
     end
 
     it "does not supports sql for model.association-virtualfield (no arel)" do
       field = "ManageIQ::Providers::InfraManager::Vm.storage-v_used_space"
-      expression = {">=" => {"field" => field, "value" => "50"}}
+      expression = {">=" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "50"}}
       expect(described_class.new(expression).sql_supports_atom?(expression)).to eq(false)
     end
 
     it "returns true for model-field" do
       field = "ManageIQ::Providers::InfraManager::Vm-vendor"
-      expression = {"=" => {"field" => field, "value" => "redhat"}}
+      expression = {"=" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "redhat"}}
       expect(described_class.new(expression).sql_supports_atom?(expression)).to eq(true)
     end
 
     it "returns true for model.assoctiation-field" do
       field = "ManageIQ::Providers::InfraManager::Vm.ext_management_system-name"
-      expression = {"STARTS WITH" => {"field" => field, "value" => "abc"}}
+      expression = {"STARTS WITH" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "abc"}}
       expect(described_class.new(expression).sql_supports_atom?(expression)).to eq(true)
     end
 
     it "returns false if column excluded from processing for adhoc performance metrics" do
       field = "EmsClusterPerformance-cpu_usagemhz_rate_average"
-      expression = {">=" => {"field" => field, "value" => "0"}}
+      expression = {">=" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "0"}}
       obj = described_class.new(expression)
       obj.preprocess_options = {:vim_performance_daily_adhoc => true}
       expect(obj.sql_supports_atom?(expression)).to eq(false)
@@ -3159,7 +3159,7 @@
 
     it "returns true if column is not excluded from processing for adhoc performance metrics" do
       field = "EmsClusterPerformance-derived_cpu_available"
-      expression = {">=" => {"field" => field, "value" => "0"}}
+      expression = {">=" => {"field" => field, "field-field" => MiqExpression::Field.parse(field), "value" => "0"}}
       obj = described_class.new(expression)
       obj.preprocess_options = {:vim_performance_daily_adhoc => true}
       expect(obj.sql_supports_atom?(expression)).to eq(true)
@@ -3170,31 +3170,31 @@
     it "returns true for model.virtualfield (with sql)" do
       field = "ManageIQ::Providers::InfraManager::Vm-archived"
       expression = {"=" => {"field" => field, "value" => "true"}}
-      expect(described_class.new(expression).field_in_sql?(field)).to eq(true)
+      expect(described_class.new(expression).field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(true)
     end
 
     it "returns false for model.virtualfield (with no sql)" do
       field = "ManageIQ::Providers::InfraManager::Vm-uncommitted_storage"
       expression = {"=" => {"field" => field, "value" => "true"}}
-      expect(described_class.new(expression).field_in_sql?(field)).to eq(false)
+      expect(described_class.new(expression).field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(false)
     end
 
     it "returns false for model.association-virtualfield" do
       field = "ManageIQ::Providers::InfraManager::Vm.storage-v_used_space_percent_of_total"
       expression = {">=" => {"field" => field, "value" => "50"}}
-      expect(described_class.new(expression).field_in_sql?(field)).to eq(false)
+      expect(described_class.new(expression).field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(false)
     end
 
     it "returns true for model-field" do
       field = "ManageIQ::Providers::InfraManager::Vm-vendor"
       expression = {"=" => {"field" => field, "value" => "redhat"}}
-      expect(described_class.new(expression).field_in_sql?(field)).to eq(true)
+      expect(described_class.new(expression).field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(true)
     end
 
     it "returns true for model.association-field" do
       field = "ManageIQ::Providers::InfraManager::Vm.guest_applications-vendor"
       expression = {"CONTAINS" => {"field" => field, "value" => "redhat"}}
-      expect(described_class.new(expression).field_in_sql?(field)).to eq(true)
+      expect(described_class.new(expression).field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(true)
     end
 
     it "returns false if column excluded from processing for adhoc performance metrics" do
@@ -3202,7 +3202,7 @@
       expression = {">=" => {"field" => field, "value" => "0"}}
       obj = described_class.new(expression)
       obj.preprocess_options = {:vim_performance_daily_adhoc => true}
-      expect(obj.field_in_sql?(field)).to eq(false)
+      expect(obj.field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(false)
     end
 
     it "returns true if column not excluded from processing for adhoc performance metrics" do
@@ -3210,7 +3210,7 @@
       expression = {">=" => {"field" => field, "value" => "0"}}
       obj = described_class.new(expression)
       obj.preprocess_options = {:vim_performance_daily_adhoc => true}
-      expect(obj.field_in_sql?(field)).to eq(true)
+      expect(obj.field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(true)
     end
   end
 

From 54211aa3c17647b7a55d3f467f8a48ca68bd9a2b Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Thu, 7 Mar 2024 12:39:24 -0500
Subject: [PATCH 5/8] dropped col_details[:excluded_by_preprocess_options,
 :format_subtype]

---
 lib/miq_expression.rb | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/lib/miq_expression.rb b/lib/miq_expression.rb
index 91176dc9542..6afa5954cf8 100644
--- a/lib/miq_expression.rb
+++ b/lib/miq_expression.rb
@@ -523,19 +523,16 @@ def sql_supports_atom?(exp)
   end
 
   def value_in_sql?(value)
-    !Field.is_field?(value) || Field.parse(value).attribute_supported_by_sql?
+    value_field = Field.parse(value)
+    !value_field&.valid? || value_field&.attribute_supported_by_sql?
   end
 
+  # NOTE: dropped cached :exclude_col. needed?
   def field_in_sql?(field, field_field)
     field_field.attribute_supported_by_sql? &&
-      !field_excluded_by_preprocess_options?(field)
+      !field_field.exclude_col_by_preprocess_options?(preprocess_options)
   end
 
-  def field_excluded_by_preprocess_options?(field)
-    col_details[field][:excluded_by_preprocess_options]
-  end
-  private :field_excluded_by_preprocess_options?
-
   def col_details
     @col_details ||= self.class.get_cols_from_expression(exp, preprocess_options)
   end
@@ -1213,8 +1210,8 @@ def self.atom_error(field, operator, value)
            if field == :count
              :integer
            else
-             col_info = get_col_info(field)
-             [:bytes, :megabytes].include?(col_info[:format_sub_type]) ? :integer : col_info[:data_type]
+             field_field = Target.parse(field)
+             [:bytes, :megabytes].include?(field_field.sub_type) ? :integer : field_field.column_type
            end
          end
 

From 2dc90f9f42b398fc8e06e40d7b01fcb454398416 Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Thu, 7 Mar 2024 13:05:56 -0500
Subject: [PATCH 6/8] introduce value-field

converge value_in_sql and field_in_sql
---
 lib/miq_expression.rb           | 18 +++++++++---------
 spec/lib/miq_expression_spec.rb | 22 +++++++++++-----------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/lib/miq_expression.rb b/lib/miq_expression.rb
index 6afa5954cf8..69db694bff0 100644
--- a/lib/miq_expression.rb
+++ b/lib/miq_expression.rb
@@ -352,6 +352,7 @@ def preprocess_exp(exp)
       field = operator_values["field"]
       field_field = operator_values["field-field"] = Field.parse(field) if field
       value = operator_values["value"]
+      operator_values["value-field"] = Field.parse(value) if value
 
       # attempt to do conversion only if db type of column is integer and value to compare to is String
       if %w[= != <= >= > <].include?(operator) && field_field&.integer? && value.instance_of?(String)
@@ -492,17 +493,17 @@ def sql_supports_atom?(exp)
     when "includes"
       # Support includes operator using "LIKE" only if first operand is in main table
       if exp[operator].key?("field") && (!exp[operator]["field"].include?(".") || (exp[operator]["field"].include?(".") && exp[operator]["field"].split(".").length == 2))
-        field_in_sql?(exp[operator]["field"], exp[operator]["field-field"])
+        field_in_sql?(exp[operator]["field-field"])
       else
         # TODO: Support includes operator for sub-sub-tables
         false
       end
     when "includes any", "includes all", "includes only"
       # Support this only from the main model (for now)
-      if exp[operator].keys.include?("field") && exp[operator]["field"].split(".").length == 1
-        model, field = exp[operator]["field"].split("-")
-        method = "miq_expression_#{operator.downcase.tr(' ', '_')}_#{field}_arel"
-        model.constantize.respond_to?(method)
+      if exp[operator]["field-field"] && exp[operator]["field"]&.split(".")&.length == 1
+        field_field = exp[operator]["field-field"]
+        method = "miq_expression_#{operator.downcase.tr(' ', '_')}_#{field_field.column}_arel"
+        field_field.model.respond_to?(method)
       else
         false
       end
@@ -518,17 +519,16 @@ def sql_supports_atom?(exp)
       # => TODO: support count of child relationship
       return false if exp[operator].key?("count")
 
-      field_in_sql?(exp[operator]["field"]) && value_in_sql?(exp[operator]["value"])
+      field_in_sql?(exp[operator]["field-field"]) && value_in_sql?(exp[operator]["value-field"])
     end
   end
 
-  def value_in_sql?(value)
-    value_field = Field.parse(value)
+  def value_in_sql?(value_field)
     !value_field&.valid? || value_field&.attribute_supported_by_sql?
   end
 
   # NOTE: dropped cached :exclude_col. needed?
-  def field_in_sql?(field, field_field)
+  def field_in_sql?(field_field)
     field_field.attribute_supported_by_sql? &&
       !field_field.exclude_col_by_preprocess_options?(preprocess_options)
   end
diff --git a/spec/lib/miq_expression_spec.rb b/spec/lib/miq_expression_spec.rb
index 018e36472a4..b9342c4be10 100644
--- a/spec/lib/miq_expression_spec.rb
+++ b/spec/lib/miq_expression_spec.rb
@@ -243,9 +243,9 @@
 
   describe "#reduce_exp" do
     let(:sql_field)      { {"=" => {"field" => "Vm-name", "value" => "foo"}.freeze}.freeze }
-    let(:sql_field_out)  { {"=" => {"field" => "Vm-name", "field-field" => MiqExpression::Field.parse("Vm-name"), "value" => "foo"}.freeze}.freeze }
+    let(:sql_field_out)  { {"=" => {"field" => "Vm-name", "field-field" => MiqExpression::Field.parse("Vm-name"), "value" => "foo", "value-field" => nil}.freeze}.freeze }
     let(:ruby_field)     { {"=" => {"field" => "Vm-platform", "value" => "bar"}.freeze}.freeze }
-    let(:ruby_field_out) { {"=" => {"field" => "Vm-platform", "field-field" => MiqExpression::Field.parse("Vm-platform"), "value" => "bar"}.freeze}.freeze }
+    let(:ruby_field_out) { {"=" => {"field" => "Vm-platform", "field-field" => MiqExpression::Field.parse("Vm-platform"), "value" => "bar", "value-field" => nil}.freeze}.freeze }
 
     context "mode: :sql" do
       it "(sql AND ruby) => (sql)" do
@@ -3017,7 +3017,7 @@
     end
   end
 
-  describe "#sql_supports_atom?" do
+  describe "#sql_supports_atom? (private)" do
     context "expression key is 'CONTAINS'" do
       context "operations with 'tag'" do
         it "returns true for tag of the main model" do
@@ -3166,35 +3166,35 @@
     end
   end
 
-  describe "#field_in_sql?" do
+  describe "#field_in_sql? (private)" do
     it "returns true for model.virtualfield (with sql)" do
       field = "ManageIQ::Providers::InfraManager::Vm-archived"
       expression = {"=" => {"field" => field, "value" => "true"}}
-      expect(described_class.new(expression).field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(true)
+      expect(described_class.new(expression).field_in_sql?(MiqExpression::Field.parse(field))).to eq(true)
     end
 
     it "returns false for model.virtualfield (with no sql)" do
       field = "ManageIQ::Providers::InfraManager::Vm-uncommitted_storage"
       expression = {"=" => {"field" => field, "value" => "true"}}
-      expect(described_class.new(expression).field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(false)
+      expect(described_class.new(expression).field_in_sql?(MiqExpression::Field.parse(field))).to eq(false)
     end
 
     it "returns false for model.association-virtualfield" do
       field = "ManageIQ::Providers::InfraManager::Vm.storage-v_used_space_percent_of_total"
       expression = {">=" => {"field" => field, "value" => "50"}}
-      expect(described_class.new(expression).field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(false)
+      expect(described_class.new(expression).field_in_sql?(MiqExpression::Field.parse(field))).to eq(false)
     end
 
     it "returns true for model-field" do
       field = "ManageIQ::Providers::InfraManager::Vm-vendor"
       expression = {"=" => {"field" => field, "value" => "redhat"}}
-      expect(described_class.new(expression).field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(true)
+      expect(described_class.new(expression).field_in_sql?(MiqExpression::Field.parse(field))).to eq(true)
     end
 
     it "returns true for model.association-field" do
       field = "ManageIQ::Providers::InfraManager::Vm.guest_applications-vendor"
       expression = {"CONTAINS" => {"field" => field, "value" => "redhat"}}
-      expect(described_class.new(expression).field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(true)
+      expect(described_class.new(expression).field_in_sql?(MiqExpression::Field.parse(field))).to eq(true)
     end
 
     it "returns false if column excluded from processing for adhoc performance metrics" do
@@ -3202,7 +3202,7 @@
       expression = {">=" => {"field" => field, "value" => "0"}}
       obj = described_class.new(expression)
       obj.preprocess_options = {:vim_performance_daily_adhoc => true}
-      expect(obj.field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(false)
+      expect(obj.field_in_sql?(MiqExpression::Field.parse(field))).to eq(false)
     end
 
     it "returns true if column not excluded from processing for adhoc performance metrics" do
@@ -3210,7 +3210,7 @@
       expression = {">=" => {"field" => field, "value" => "0"}}
       obj = described_class.new(expression)
       obj.preprocess_options = {:vim_performance_daily_adhoc => true}
-      expect(obj.field_in_sql?(field, MiqExpression::Field.parse(field))).to eq(true)
+      expect(obj.field_in_sql?(MiqExpression::Field.parse(field))).to eq(true)
     end
   end
 

From 1378481be7eae7351879d18daab7db4afd166a79 Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Mon, 15 Apr 2024 15:37:21 -0400
Subject: [PATCH 7/8] MiqExpression#preprocess_exp for more cases

still work even if :token is the first key in the exp
still process find sub targets

this gets us more values with field-field
---
 lib/miq_expression.rb | 68 +++++++++++++++++++++++--------------------
 1 file changed, 37 insertions(+), 31 deletions(-)

diff --git a/lib/miq_expression.rb b/lib/miq_expression.rb
index 69db694bff0..79058b96147 100644
--- a/lib/miq_expression.rb
+++ b/lib/miq_expression.rb
@@ -303,19 +303,16 @@ def self._to_ruby(exp, context_type, tz)
       clause, = operands2rubyvalue(operator, op_args, context_type)
     when "is"
       col_ruby, _value = operands2rubyvalue(operator, {"field" => col_name}, context_type)
-      col_type = Target.parse(col_name).column_type
       value = op_args["value"]
-      clause = if col_type == :date && !RelativeDatetime.relative?(value)
-                 ruby_for_date_compare(col_ruby, col_type, tz, "==", value)
+      clause = if field.date? && !RelativeDatetime.relative?(value)
+                 ruby_for_date_compare(col_ruby, field.column_type, tz, "==", value)
                else
-                 ruby_for_date_compare(col_ruby, col_type, tz, ">=", value, "<=", value)
+                 ruby_for_date_compare(col_ruby, field.column_type, tz, ">=", value, "<=", value)
                end
     when "from"
       col_ruby, _value = operands2rubyvalue(operator, {"field" => col_name}, context_type)
-      col_type = Target.parse(col_name).column_type
-
       start_val, end_val = op_args["value"]
-      clause = ruby_for_date_compare(col_ruby, col_type, tz, ">=", start_val, "<=", end_val)
+      clause = ruby_for_date_compare(col_ruby, field.column_type, tz, ">=", start_val, "<=", end_val)
     else
       raise _("operator '%{operator_name}' is not supported") % {:operator_name => operator.upcase}
     end
@@ -335,31 +332,40 @@ def to_sql(tz = nil)
   end
 
   def preprocess_exp(exp)
-    operator = exp.keys.first
-    operator_values = exp[operator]
-    case operator.downcase
-    when "and", "or"
-      operator_values = operator_values.map { |atom| preprocess_exp(atom) }
-    when "not", "!"
-      operator_values = preprocess_exp(operator_values)
-    else # field
-      # op => {"regkey"=>"foo", "regval"=>"bar", "value"=>"baz"}
-      # op => {"field" => "foo", "value" => "baz"}
-      # op => {"field" => "<count>, "value" => "0"}
-      # op => {"count" => "Vm.snapshots", "value"=>"1"}
-      # op => {"tag"=>"Host.managed-environment", "value"=>"prod"}
-      operator_values = operator_values.dup
-      field = operator_values["field"]
-      field_field = operator_values["field-field"] = Field.parse(field) if field
-      value = operator_values["value"]
-      operator_values["value-field"] = Field.parse(value) if value
-
-      # attempt to do conversion only if db type of column is integer and value to compare to is String
-      if %w[= != <= >= > <].include?(operator) && field_field&.integer? && value.instance_of?(String)
-        operator_values["value"] = convert_size_in_units_to_integer(field, field_field.sub_type, value)
-      end
+    exp.each_with_object({}) do |(operator, operator_values), new_exp|
+      next if operator == :token
+
+      new_exp[operator] =
+        case operator.downcase
+        when "and", "or"
+          # "and" => [exp, exp, exp]
+          operator_values.map { |atom| preprocess_exp(atom) }
+        when "not", "!"
+          # "not" => exp
+          preprocess_exp(operator_values)
+        when "find"
+          # "find" => {"search" => exp, "checkall" => exp}
+          operator_values.each_with_object({}) { |(op2, op_values2), hash| hash[op2] = preprocess_exp(op_values2) }
+        else # field
+          # op => {"regkey"=>"foo", "regval"=>"bar", "value"=>"baz"}
+          # op => {"field" => "foo", "value" => "baz"}
+          # op => {"field" => "<count>, "value" => "0"}
+          # op => {"count" => "Vm.snapshots", "value"=>"1"}
+          # op => {"tag"=>"Host.managed-environment", "value"=>"prod"}
+          operator_values = operator_values.dup
+          field = operator_values["field"]
+          field_field = operator_values["field-field"] = Field.parse(field) if field
+          value = operator_values["value"]
+          operator_values["value-field"] = Field.parse(value) if value
+
+          # attempt to do conversion only if db type of column is integer and value to compare to is String
+          if %w[= != <= >= > <].include?(operator) && field_field&.integer? && value.instance_of?(String)
+            operator_values["value"] = convert_size_in_units_to_integer(field, field_field.sub_type, value)
+          end
+
+          operator_values
+        end
     end
-    {operator => operator_values}
   end
 
   # @param operator [String]      operator (i.e.: AND, OR, NOT)

From ddf44703044041f55d6859e6552ebdcef19063ae Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Mon, 15 Apr 2024 15:36:17 -0400
Subject: [PATCH 8/8] move valid? into preprocess_exp

Think this was the only reference to MiqExpressoin#valid?
---
 lib/miq_expression.rb | 48 +++++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/lib/miq_expression.rb b/lib/miq_expression.rb
index 79058b96147..73f4dd306a6 100644
--- a/lib/miq_expression.rb
+++ b/lib/miq_expression.rb
@@ -1,4 +1,6 @@
 class MiqExpression
+  class InvalidExpression < StandardError; end
+
   # bit array of the types of nodes available/desired
   MODE_NONE = 0
   MODE_RUBY = 1
@@ -35,28 +37,11 @@ def initialize(exp, ctype = nil)
     @ruby = nil
   end
 
-  def valid?(component = exp)
-    operator = component.keys.first
-    case operator.downcase
-    when "and", "or"
-      component[operator].all?(&method(:valid?))
-    when "not", "!"
-      valid?(component[operator])
-    when "find"
-      validate_set = Set.new(%w[checkall checkany checkcount search])
-      validate_keys = component[operator].keys.select { |k| validate_set.include?(k) }
-      validate_keys.all? { |k| valid?(component[operator][k]) }
-    else
-      if component[operator].key?("field")
-        field = Field.parse(component[operator]["field"])
-        return false if field && !field.valid?
-      end
-      if Field.is_field?(component[operator]["value"])
-        field = Field.parse(component[operator]["value"])
-        return false unless field && field.valid?
-      end
-      true
-    end
+  def valid?
+    preprocess_exp(exp)
+    true
+  rescue InvalidExpression
+    false
   end
 
   def set_tagged_target(model, associations = [])
@@ -180,14 +165,14 @@ def to_ruby(timezone = nil, prune_sql: false)
       nil
     elsif @ruby
       @ruby.dup
-    elsif valid?
+    else
       pexp = preprocess_exp(exp)
       pexp, _ = prune_exp(pexp, MODE_RUBY) if prune_sql
       @ruby = self.class._to_ruby(pexp, context_type, timezone) || true
       @ruby == true ? nil : @ruby.dup
-    else
-      ""
     end
+  rescue InvalidExpression
+    ""
   end
 
   def self._to_ruby(exp, context_type, tz)
@@ -329,6 +314,8 @@ def to_sql(tz = nil)
     sql = to_arel(pexp, tz).to_sql if pexp.present?
     incl = includes_for_sql if sql.present?
     [sql, incl, attrs]
+  rescue InvalidExpression
+    [nil, nil, {:supported_by_sql => false}]
   end
 
   def preprocess_exp(exp)
@@ -351,12 +338,19 @@ def preprocess_exp(exp)
           # op => {"field" => "foo", "value" => "baz"}
           # op => {"field" => "<count>, "value" => "0"}
           # op => {"count" => "Vm.snapshots", "value"=>"1"}
+          # op => {"field" => "<count>", "value"=>"1"}
           # op => {"tag"=>"Host.managed-environment", "value"=>"prod"}
           operator_values = operator_values.dup
           field = operator_values["field"]
-          field_field = operator_values["field-field"] = Field.parse(field) if field
+          if field && field != "<count>"
+            field_field = operator_values["field-field"] = Field.parse(field)
+            raise(InvalidExpression, field) unless field_field.valid?
+          end
           value = operator_values["value"]
-          operator_values["value-field"] = Field.parse(value) if value
+          if value
+            value_field = operator_values["value-field"] = Field.parse(value)
+            raise(InvalidExpression, field) if value_field && !value_field.valid?
+          end
 
           # attempt to do conversion only if db type of column is integer and value to compare to is String
           if %w[= != <= >= > <].include?(operator) && field_field&.integer? && value.instance_of?(String)