Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

relation .present? and .blank? should not query SELECT COUNT(DISTINCT id) #5461

Merged
merged 1 commit into from

2 participants

@thejamespinto

It's a good practice to write "if @post.present?" instead of "if @posts", because it will be more readable.
however, if we are using lists, and say "if @posts.blank?" it will trigger a SELECT COUNT,
not only that, if you use JOINS, it will trigger "SELECT COUNT(DISTINCT 'posts'.id)"
that meaningless query will only affect the performance of the overall request.

Programmers know if they invoke .count or .exists? they will trigger these smaller queries "on purpose"
but methods blank? and present? invoke .empty? internally, which gets overwritten by relations.

I have seen this common miss-usage of a good practice in 02 legacy apps that I've worked already,
and I believe, it would be a good improvement in Rails if those methods (that are commonly used for checks in the view layer to display a custom message for empty), would trigger the SQL query directly, thus not invoking a COUNT DISTINCT just for checking.

@thejamespinto

In a single request, where I found 05 examples of @sales.blank? in the view, I was able to reduce the load in ~45 seconds in a complex query with joins, that were invoking SELECT COUNT by a programmer's erroneous attempt to do well-written code.

@josevalim josevalim merged commit fa7a3aa into rails:master
@josevalim
Owner

Could you also add a test case please? Maybe you could use a method called assert_queries to ensure just one query is done after a @posts.blank?; @posts.to_a call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 4 additions and 0 deletions.
  1. +4 −0 activerecord/lib/active_record/relation.rb
View
4 activerecord/lib/active_record/relation.rb
@@ -507,6 +507,10 @@ def with_default_scope #:nodoc:
end
end
+ def blank?
+ to_a.blank?
+ end
+
private
def references_eager_loaded_tables?
Something went wrong with that request. Please try again.