jro_apps/redmine Commit - r12206:77ac3b7891d4

Reduces the number of subqueries when searching with many custom fields set as searchable (#15781)....

Jean-Philippe Lang -

r12206:77ac3b7891d4

parent child

Context file:

r12206:77ac3b7891d4

Collapse all files

app/models/custom_field.rb +3 -2

             # Redmine - project management software
             # Copyright (C) 2006-2013  Jean-Philippe Lang
             #
             # This program is free software; you can redistribute it and/or
             # modify it under the terms of the GNU General Public License
             # as published by the Free Software Foundation; either version 2
             # of the License, or (at your option) any later version.
             #
             # This program is distributed in the hope that it will be useful,
             # but WITHOUT ANY WARRANTY; without even the implied warranty of
             # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
             # GNU General Public License for more details.
             #
             # You should have received a copy of the GNU General Public License
             # along with this program; if not, write to the Free Software
             # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
             class CustomField < ActiveRecord::Base
               include Redmine::SubclassFactory
               has_many :custom_values, :dependent => :delete_all
               has_and_belongs_to_many :roles, :join_table => "#{table_name_prefix}custom_fields_roles#{table_name_suffix}", :foreign_key => "custom_field_id"
               acts_as_list :scope => 'type = \'#{self.class}\''
               serialize :possible_values
               store :format_store
               validates_presence_of :name, :field_format
               validates_uniqueness_of :name, :scope => :type
               validates_length_of :name, :maximum => 30
               validates_inclusion_of :field_format, :in => Proc.new { Redmine::FieldFormat.available_formats }
               validate :validate_custom_field
               before_validation :set_searchable
               before_save do |field|
                 field.format.before_custom_field_save(field)
               end
               after_save :handle_multiplicity_change
               after_save do |field|
                 if field.visible_changed? && field.visible
                   field.roles.clear
                 end
               end
               scope :sorted, lambda { order("#{table_name}.position ASC") }
               scope :visible, lambda {|*args|
                 user = args.shift || User.current
                 if user.admin?
                   # nop
                 elsif user.memberships.any?
                   where("#{table_name}.visible = ? OR #{table_name}.id IN (SELECT DISTINCT cfr.custom_field_id FROM #{Member.table_name} m" +
                     " INNER JOIN #{MemberRole.table_name} mr ON mr.member_id = m.id" +
                     " INNER JOIN #{table_name_prefix}custom_fields_roles#{table_name_suffix} cfr ON cfr.role_id = mr.role_id" +
                     " WHERE m.user_id = ?)",
                     true, user.id)
                 else
                   where(:visible => true)
                 end
               }
               def visible_by?(project, user=User.current)
                 visible? || user.admin?
               end
               def format
                 @format ||= Redmine::FieldFormat.find(field_format)
               end
               def field_format=(arg)
                 # cannot change format of a saved custom field
                 if new_record?
                   @format = nil
                   super
                 end
               end
               def set_searchable
                 # make sure these fields are not searchable
                 self.searchable = false unless format.class.searchable_supported
                 # make sure only these fields can have multiple values
                 self.multiple = false unless format.class.multiple_supported
                 true
               end
               def validate_custom_field
                 format.validate_custom_field(self).each do |attribute, message|
                   errors.add attribute, message
                 end
                 if regexp.present?
                   begin
                     Regexp.new(regexp)
                   rescue
                     errors.add(:regexp, :invalid)
                   end
                 end
                 if default_value.present?
                   validate_field_value(default_value).each do |message|
                     errors.add :default_value, message
                   end
                 end
               end
               def possible_custom_value_options(custom_value)
                 format.possible_custom_value_options(custom_value)
               end
               def possible_values_options(object=nil)
                 if object.is_a?(Array)
                   object.map {|o| format.possible_values_options(self, o)}.reduce(:&) || []
                 else
                   format.possible_values_options(self, object) || []
                 end
               end
               def possible_values
                 values = super()
                 if values.is_a?(Array)
                   values.each do |value|
                     value.force_encoding('UTF-8') if value.respond_to?(:force_encoding)
                   end
                   values
                 else
                   []
                 end
               end
               # Makes possible_values accept a multiline string
               def possible_values=(arg)
                 if arg.is_a?(Array)
                   super(arg.compact.collect(&:strip).select {|v| !v.blank?})
                 else
                   self.possible_values = arg.to_s.split(/[\n\r]+/)
                 end
               end
               def cast_value(value)
                 format.cast_value(self, value)
               end
               def value_from_keyword(keyword, customized)
                 possible_values_options = possible_values_options(customized)
                 if possible_values_options.present?
                   keyword = keyword.to_s.downcase
                   if v = possible_values_options.detect {|text, id| text.downcase == keyword}
                     if v.is_a?(Array)
                       v.last
                     else
                       v
                     end
                   end
                 else
                   keyword
                 end
               end
               # Returns a ORDER BY clause that can used to sort customized
               # objects by their value of the custom field.
               # Returns nil if the custom field can not be used for sorting.
               def order_statement
                 return nil if multiple?
                 format.order_statement(self)
               end
               # Returns a GROUP BY clause that can used to group by custom value
               # Returns nil if the custom field can not be used for grouping.
               def group_statement
                 return nil if multiple?
                 format.group_statement(self)
               end
               def join_for_order_statement
                 format.join_for_order_statement(self)
               end
-              def visibility_by_project_condition(project_key=nil, user=User.current)
+              def visibility_by_project_condition(project_key=nil, user=User.current, id_column=nil)
                 if visible? || user.admin?
                   "1=1"
                 elsif user.anonymous?
                   "1=0"
                 else
                   project_key ||= "#{self.class.customized_class.table_name}.project_id"
+                  id_column ||= id
                   "#{project_key} IN (SELECT DISTINCT m.project_id FROM #{Member.table_name} m" +
                     " INNER JOIN #{MemberRole.table_name} mr ON mr.member_id = m.id" +
                     " INNER JOIN #{table_name_prefix}custom_fields_roles#{table_name_suffix} cfr ON cfr.role_id = mr.role_id" +
-                    " WHERE m.user_id = #{user.id} AND cfr.custom_field_id = #{id})"
+                    " WHERE m.user_id = #{user.id} AND cfr.custom_field_id = #{id_column})"
                 end
               end
               def self.visibility_condition
                 if user.admin?
                   "1=1"
                 elsif user.anonymous?
                   "#{table_name}.visible"
                 else
                   "#{project_key} IN (SELECT DISTINCT m.project_id FROM #{Member.table_name} m" +
                     " INNER JOIN #{MemberRole.table_name} mr ON mr.member_id = m.id" +
                     " INNER JOIN #{table_name_prefix}custom_fields_roles#{table_name_suffix} cfr ON cfr.role_id = mr.role_id" +
                     " WHERE m.user_id = #{user.id} AND cfr.custom_field_id = #{id})"
                 end
               end
               def <=>(field)
                 position <=> field.position
               end
               # Returns the class that values represent
               def value_class
                 format.target_class if format.respond_to?(:target_class)
               end
               def self.customized_class
                 self.name =~ /^(.+)CustomField$/
                 $1.constantize rescue nil
               end
               # to move in project_custom_field
               def self.for_all
                 where(:is_for_all => true).order('position').all
               end
               def type_name
                 nil
               end
               # Returns the error messages for the given value
               # or an empty array if value is a valid value for the custom field
               def validate_custom_value(custom_value)
                 value = custom_value.value
                 errs = []
                 if value.is_a?(Array)
                   if !multiple?
                     errs << ::I18n.t('activerecord.errors.messages.invalid')
                   end
                   if is_required? && value.detect(&:present?).nil?
                     errs << ::I18n.t('activerecord.errors.messages.blank')
                   end
                 else
                   if is_required? && value.blank?
                     errs << ::I18n.t('activerecord.errors.messages.blank')
                   end
                 end
                 if custom_value.value.present?
                   errs += format.validate_custom_value(custom_value)
                 end
                 errs
               end
               # Returns the error messages for the default custom field value
               def validate_field_value(value)
                 validate_custom_value(CustomValue.new(:custom_field => self, :value => value))
               end
               # Returns true if value is a valid value for the custom field
               def valid_field_value?(value)
                 validate_field_value(value).empty?
               end
               def format_in?(*args)
                 args.include?(field_format)
               end
               protected
               # Removes multiple values for the custom field after setting the multiple attribute to false
               # We kepp the value with the highest id for each customized object
               def handle_multiplicity_change
                 if !new_record? && multiple_was && !multiple
                   ids = custom_values.
                     where("EXISTS(SELECT 1 FROM #{CustomValue.table_name} cve WHERE cve.custom_field_id = #{CustomValue.table_name}.custom_field_id" +
                       " AND cve.customized_type = #{CustomValue.table_name}.customized_type AND cve.customized_id = #{CustomValue.table_name}.customized_id" +
                       " AND cve.id > #{CustomValue.table_name}.id)").
                     pluck(:id)
                   if ids.any?
                     custom_values.where(:id => ids).delete_all
                   end
                 end
               end
             end

app/models/issue_custom_field.rb +7 -6

             # Redmine - project management software
             # Copyright (C) 2006-2013  Jean-Philippe Lang
             #
             # This program is free software; you can redistribute it and/or
             # modify it under the terms of the GNU General Public License
             # as published by the Free Software Foundation; either version 2
             # of the License, or (at your option) any later version.
             #
             # This program is distributed in the hope that it will be useful,
             # but WITHOUT ANY WARRANTY; without even the implied warranty of
             # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
             # GNU General Public License for more details.
             #
             # You should have received a copy of the GNU General Public License
             # along with this program; if not, write to the Free Software
             # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
             class IssueCustomField < CustomField
               has_and_belongs_to_many :projects, :join_table => "#{table_name_prefix}custom_fields_projects#{table_name_suffix}", :foreign_key => "custom_field_id"
               has_and_belongs_to_many :trackers, :join_table => "#{table_name_prefix}custom_fields_trackers#{table_name_suffix}", :foreign_key => "custom_field_id"
               has_many :issues, :through => :issue_custom_values
               def type_name
                 :label_issue_plural
               end
               def visible_by?(project, user=User.current)
                 super || (roles & user.roles_for_project(project)).present?
               end
-              def visibility_by_project_condition(*args)
+              def visibility_by_project_condition(project_key=nil, user=User.current, id_column=nil)
                 sql = super
-                additional_sql = "#{Issue.table_name}.tracker_id IN (SELECT tracker_id FROM #{table_name_prefix}custom_fields_trackers#{table_name_suffix} WHERE custom_field_id = #{id})"
+                id_column ||= id
-                unless is_for_all?
+                tracker_condition = "#{Issue.table_name}.tracker_id IN (SELECT tracker_id FROM #{table_name_prefix}custom_fields_trackers#{table_name_suffix} WHERE custom_field_id = #{id_column})"
-                  additional_sql << " AND #{Issue.table_name}.project_id IN (SELECT project_id FROM #{table_name_prefix}custom_fields_projects#{table_name_suffix} WHERE custom_field_id = #{id})"
+                project_condition = "EXISTS (SELECT 1 FROM #{CustomField.table_name} ifa WHERE ifa.is_for_all = #{connection.quoted_true} AND ifa.id = #{id_column})" +
-                end
+                  " OR #{Issue.table_name}.project_id IN (SELECT project_id FROM #{table_name_prefix}custom_fields_projects#{table_name_suffix} WHERE custom_field_id = #{id_column})"
-                "((#{sql}) AND (#{additional_sql}))"
+                "((#{sql}) AND (#{tracker_condition}) AND (#{project_condition}))"
               end
               def validate_custom_field
                 super
                 errors.add(:base, l(:label_role_plural) + ' ' + l('activerecord.errors.messages.blank')) unless visible? || roles.present?
               end
             end

lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb +10 -5

             # Redmine - project management software
             # Copyright (C) 2006-2013  Jean-Philippe Lang
             #
             # This program is free software; you can redistribute it and/or
             # modify it under the terms of the GNU General Public License
             # as published by the Free Software Foundation; either version 2
             # of the License, or (at your option) any later version.
             #
             # This program is distributed in the hope that it will be useful,
             # but WITHOUT ANY WARRANTY; without even the implied warranty of
             # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
             # GNU General Public License for more details.
             #
             # You should have received a copy of the GNU General Public License
             # along with this program; if not, write to the Free Software
             # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
             module Redmine
               module Acts
                 module Searchable
                   def self.included(base)
                     base.extend ClassMethods
                   end
                   module ClassMethods
                     # Options:
                     # * :columns - a column or an array of columns to search
                     # * :project_key - project foreign key (default to project_id)
                     # * :date_column - name of the datetime column (default to created_on)
                     # * :sort_order - name of the column used to sort results (default to :date_column or created_on)
                     # * :permission - permission required to search the model (default to :view_"objects")
                     def acts_as_searchable(options = {})
                       return if self.included_modules.include?(Redmine::Acts::Searchable::InstanceMethods)
                       cattr_accessor :searchable_options
                       self.searchable_options = options
                       if searchable_options[:columns].nil?
                         raise 'No searchable column defined.'
                       elsif !searchable_options[:columns].is_a?(Array)
                         searchable_options[:columns] = [] << searchable_options[:columns]
                       end
                       searchable_options[:project_key] ||= "#{table_name}.project_id"
                       searchable_options[:date_column] ||= "#{table_name}.created_on"
                       searchable_options[:order_column] ||= searchable_options[:date_column]
                       # Should we search custom fields on this model ?
                       searchable_options[:search_custom_fields] = !reflect_on_association(:custom_values).nil?
                       send :include, Redmine::Acts::Searchable::InstanceMethods
                     end
                   end
                   module InstanceMethods
                     def self.included(base)
                       base.extend ClassMethods
                     end
                     module ClassMethods
                       # Searches the model for the given tokens
                       # projects argument can be either nil (will search all projects), a project or an array of projects
                       # Returns the results and the results count
                       def search(tokens, projects=nil, options={})
                         if projects.is_a?(Array) && projects.empty?
                           # no results
                           return [[], 0]
                         end
                         # TODO: make user an argument
                         user = User.current
                         tokens = [] << tokens unless tokens.is_a?(Array)
                         projects = [] << projects unless projects.nil? || projects.is_a?(Array)
                         limit_options = {}
                         limit_options[:limit] = options[:limit] if options[:limit]
                         columns = searchable_options[:columns]
                         columns = columns[0..0] if options[:titles_only]
                         token_clauses = columns.collect {|column| "(LOWER(#{column}) LIKE ?)"}
                         if !options[:titles_only] && searchable_options[:search_custom_fields]
                           searchable_custom_fields = CustomField.where(:type => "#{self.name}CustomField", :searchable => true)
-                          searchable_custom_fields.each do |field|
+                          fields_by_visibility = searchable_custom_fields.group_by {|field|
-                            sql = "#{table_name}.id IN (SELECT customized_id FROM #{CustomValue.table_name}" +
+                            field.visibility_by_project_condition(searchable_options[:project_key], user, "cfs.custom_field_id")
-                              " WHERE customized_type='#{self.name}' AND customized_id=#{table_name}.id AND LOWER(value) LIKE ?" +
-                              " AND #{CustomValue.table_name}.custom_field_id = #{field.id})" +
+                          # only 1 subquery for all custom fields with the same visibility statement
-                              " AND #{field.visibility_by_project_condition(searchable_options[:project_key], user)}"
+                          fields_by_visibility.each do |visibility, fields|
+                            ids = fields.map(&:id).join(',')
+                            sql = "#{table_name}.id IN (SELECT cfs.customized_id FROM #{CustomValue.table_name} cfs" +
+                              " WHERE cfs.customized_type='#{self.name}' AND cfs.customized_id=#{table_name}.id AND LOWER(cfs.value) LIKE ?" +
+                              " AND cfs.custom_field_id IN (#{ids})" +
+                              " AND #{visibility})"
                             token_clauses << sql
                           end
                         end
                         sql = (['(' + token_clauses.join(' OR ') + ')'] * tokens.size).join(options[:all_words] ? ' AND ' : ' OR ')
                         tokens_conditions = [sql, * (tokens.collect {|w| "%#{w.downcase}%"} * token_clauses.size).sort]
                         scope = self.scoped
                         project_conditions = []
                         if searchable_options.has_key?(:permission)
                           project_conditions << Project.allowed_to_condition(user, searchable_options[:permission] || :view_project)
                         elsif respond_to?(:visible)
                           scope = scope.visible(user)
                         else
                           ActiveSupport::Deprecation.warn "acts_as_searchable with implicit :permission option is deprecated. Add a visible scope to the #{self.name} model or use explicit :permission option."
                           project_conditions << Project.allowed_to_condition(user, "view_#{self.name.underscore.pluralize}".to_sym)
                         end
                         # TODO: use visible scope options instead
                         project_conditions << "#{searchable_options[:project_key]} IN (#{projects.collect(&:id).join(',')})" unless projects.nil?
                         project_conditions = project_conditions.empty? ? nil : project_conditions.join(' AND ')
                         results = []
                         results_count = 0
                         scope = scope.
                           includes(searchable_options[:include]).
                           order("#{searchable_options[:order_column]} " + (options[:before] ? 'DESC' : 'ASC')).
                           where(project_conditions).
                           where(tokens_conditions)
                         results_count = scope.count
                         scope_with_limit = scope.limit(options[:limit])
                         if options[:offset]
                           scope_with_limit = scope_with_limit.where("#{searchable_options[:date_column]} #{options[:before] ? '<' : '>'} ?", options[:offset])
                         end
                         results = scope_with_limit.all
                         [results, results_count]
                       end
                     end
                   end
                 end
               end
             end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages