Skip to content

Commit

Permalink
Merge pull request #18649 from yrudman/convert-human-sizes-to-bytes-i…
Browse files Browse the repository at this point in the history
…n-expression

Fix: convert string representation of sizes to numbers when generating SQL for expression
(cherry picked from commit db74833)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1702077
  • Loading branch information
gtanzillo authored and simaishi committed Apr 22, 2019
1 parent 9abba2b commit 0046a86
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 3 deletions.
28 changes: 26 additions & 2 deletions lib/miq_expression.rb
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,11 @@ def preprocess_for_sql(exp, attrs = nil)
preprocess_for_sql(exp[operator], attrs)
exp.delete(operator) if exp[operator].empty? # Clean out empty operands
else
# check operands to see if they can be represented in sql
unless sql_supports_atom?(exp)
if sql_supports_atom?(exp)
# if field type is Integer and value is String representing size in units (like "2.megabytes") than convert
# this string to correct number using sub_type mappong defined in db/fixtures/miq_report_formats.yml:sub_types_by_column:
convert_size_in_units_to_integer(exp) if %w[= != <= >= > <].include?(operator)
else
attrs[:supported_by_sql] = false
exp.delete(operator)
end
Expand Down Expand Up @@ -1300,6 +1303,27 @@ 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"]).class == 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
when :bytes
exp.values.first["value"] = value.to_i_with_method
when :kilobytes
exp.values.first["value"] = value.to_i_with_method / 1_024
when :megabytes, :megabytes_precision_2
exp.values.first["value"] = 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'")
end
end

# example:
# ruby_for_date_compare(:updated_at, :date, tz, "==", Time.now)
# # => "val=update_at; !val.nil? && val.to_date == '2016-10-05'"
Expand Down
21 changes: 20 additions & 1 deletion spec/lib/miq_expression_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

it 'lists custom attributes in ChargebackVm' do
skip('removing of virtual custom attributes is needed to do first in other specs')

displayed_columms = described_class.reporting_available_fields('ChargebackVm').map(&:second)
expected_columns = (ChargebackVm.attribute_names - extra_fields).map { |x| "ChargebackVm-#{x}" }

Expand Down Expand Up @@ -222,6 +222,25 @@
end
end

describe "#preprocess_for_sql" 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_for_sql(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_for_sql(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_for_sql(expession_hash)
expect(exp.values.first["value"]).to eq(2.terabytes.to_i_with_method)
end
end

describe "#to_sql" do
it "generates the SQL for an EQUAL expression" do
sql, * = MiqExpression.new("EQUAL" => {"field" => "Vm-name", "value" => "foo"}).to_sql
Expand Down

0 comments on commit 0046a86

Please sign in to comment.