Fix #2246 - take2: make sure we run the rails tag query only when needed

Adding the tags to the rails collect query can reduce performance
because there are 2 more tables to join with.
So we make sure to include tags in the query only when it is
necessary.

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

2 files changed, 19 insertions, 2 deletions

diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/collector.rb
index 5f11a6cea..a7f81b4e7 100644
--- a/lib/puppet/parser/collector.rb
+++ b/lib/puppet/parser/collector.rb
@@ -102,13 +102,20 @@ class Puppet::Parser::Collector
         raise Puppet::DevError, "Cannot collect resources for a nil host" unless @scope.host
         host = Puppet::Rails::Host.find_by_name(@scope.host)
 
-        query = {:include => {:param_values => :param_name, :puppet_tags => :resource_tags}}
+        query = {:include => {:param_values => :param_name}}
 
         search = "(exported=? AND restype=?)"
         values = [true, @type]
 
         search += " AND (%s)" % @equery if @equery
 
+        # this is a small performance optimisation
+        # the tag mechanism involves 3 more joins, which are
+        # needed only if we query on tags.
+        if search =~ /puppet_tags/
+            query[:include][:puppet_tags] = :resource_tags
+        end
+
         # We're going to collect objects from rails, but we don't want any
         # objects from this host.
         search = ("host_id != ? AND %s" % search) and values.unshift(host.id) if host
diff --git a/spec/unit/parser/collector.rb b/spec/unit/parser/collector.rb
index c2d5a991b..926033c68 100755
--- a/spec/unit/parser/collector.rb
+++ b/spec/unit/parser/collector.rb
@@ -505,7 +505,17 @@ describe Puppet::Parser::Collector, "when building its ActiveRecord query for co
         @collector.evaluate.should == [@resource]
     end
 
-    it "should return parameter names, parameter values and tags when querying ActiveRecord" do
+    it "should return parameter names, parameter values when querying ActiveRecord" do
+        Puppet::Rails::Resource.stubs(:find).with { |*arguments|
+            options = arguments[3]
+            options[:include] == {:param_values => :param_name}
+        }.returns([@resource])
+
+        @collector.evaluate.should == [@resource]
+    end
+
+    it "should return tags when querying ActiveRecord with a tag exported query" do
+        @collector.equery = "puppet_tags.name = test"
         Puppet::Rails::Resource.stubs(:find).with { |*arguments|
             options = arguments[3]
             options[:include] == {:param_values => :param_name, :puppet_tags => :resource_tags}