Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: convert string representation of sizes to numbers when generating SQL for expression #18649

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1279,6 +1282,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