Skip to content

Commit

Permalink
Prefer .exists? over .length? in report button helpers
Browse files Browse the repository at this point in the history
https://bugzilla.redhat.com/show_bug.cgi?id=1590908

Calls to `.length` will fetch the entire record set or rows for the
MiqReportResult, and in the current case of these button helpers, throw
them all away without using any of the data fetched.  In most cases, the
`html_details` relation is probably what was is being used, so even
caching these ahead of time is wasteful.

By using `.exists?` as an alternative, we basically do a:

  SELECT 1 as one
  FROM miq_report_result_details
  WHERE "miq_report_result_details"."miq_report_result_id" = ?
  LIMIT 1

Which only returns a single digit from the database if it has at least
one record and nothing if it doesn't.  This will also be cached in a
controller action by the ActiveRecord query cache, so it will also
require zero queries the second time around.
  • Loading branch information
NickLaMuro committed Jun 19, 2018
1 parent b5260fd commit cb118f0
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
@@ -1,5 +1,5 @@
class ApplicationHelper::Button::ReportDownloadChoice < ApplicationHelper::Button::Basic
def disabled?
MiqReportResult.find(@report_result_id).try(:miq_report_result_details).try(:length).to_i == 0
!MiqReportResult.find(@report_result_id).try(:miq_report_result_details).try(:exists?)
end
end
2 changes: 1 addition & 1 deletion app/helpers/application_helper/button/report_only.rb
Expand Up @@ -11,6 +11,6 @@ def disabled?

def report_records?
@report.present? && @report_result_id.present? &&
MiqReportResult.find(@report_result_id).try(:miq_report_result_details).try(:length).to_i > 0
MiqReportResult.find(@report_result_id).try(:miq_report_result_details).try(:exists?)
end
end

0 comments on commit cb118f0

Please sign in to comment.