From d82159bcf544bb66e301599ae0cada1ce73f2335 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Thu, 3 Jan 2013 12:28:50 +0000 Subject: [PATCH] Use joins instead of sub-queries in group by/sort by when using custom fields (#12713). Sub-queries in group by are not supported by SQLServer. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11102 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/custom_field.rb | 34 ++++++++++++++++++++++------------ app/models/issue_query.rb | 2 +- test/unit/query_test.rb | 12 +++--------- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 40582ff73..a9c42d4bb 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -180,18 +180,12 @@ class CustomField < ActiveRecord::Base case field_format when 'string', 'text', 'list', 'date', 'bool' # COALESCE is here to make sure that blank and NULL values are sorted equally - "COALESCE((SELECT cv_sort.value FROM #{CustomValue.table_name} cv_sort" + - " WHERE cv_sort.customized_type='#{self.class.customized_class.base_class.name}'" + - " AND cv_sort.customized_id=#{self.class.customized_class.table_name}.id" + - " AND cv_sort.custom_field_id=#{id} LIMIT 1), '')" + "COALESCE(#{join_alias}.value, '')" when 'int', 'float' # Make the database cast values into numeric # Postgresql will raise an error if a value can not be casted! # CustomValue validations should ensure that it doesn't occur - "(SELECT CAST(cv_sort.value AS decimal(30,3)) FROM #{CustomValue.table_name} cv_sort" + - " WHERE cv_sort.customized_type='#{self.class.customized_class.base_class.name}'" + - " AND cv_sort.customized_id=#{self.class.customized_class.table_name}.id" + - " AND cv_sort.custom_field_id=#{id} AND cv_sort.value <> '' AND cv_sort.value IS NOT NULL LIMIT 1)" + "CAST(#{join_alias}.value AS decimal(30,3))" when 'user', 'version' value_class.fields_for_order_statement(value_join_alias) else @@ -207,10 +201,7 @@ class CustomField < ActiveRecord::Base when 'list', 'date', 'bool', 'int' order_statement when 'user', 'version' - "COALESCE((SELECT cv_sort.value FROM #{CustomValue.table_name} cv_sort" + - " WHERE cv_sort.customized_type='#{self.class.customized_class.base_class.name}'" + - " AND cv_sort.customized_id=#{self.class.customized_class.table_name}.id" + - " AND cv_sort.custom_field_id=#{id} LIMIT 1), '')" + "COALESCE(#{join_alias}.value, '')" else nil end @@ -230,6 +221,25 @@ class CustomField < ActiveRecord::Base " AND #{join_alias}_2.custom_field_id = #{join_alias}.custom_field_id)" + " LEFT OUTER JOIN #{value_class.table_name} #{value_join_alias}" + " ON CAST(#{join_alias}.value as decimal(30,0)) = #{value_join_alias}.id" + when 'int', 'float' + "LEFT OUTER JOIN #{CustomValue.table_name} #{join_alias}" + + " ON #{join_alias}.customized_type = '#{self.class.customized_class.base_class.name}'" + + " AND #{join_alias}.customized_id = #{self.class.customized_class.table_name}.id" + + " AND #{join_alias}.custom_field_id = #{id}" + + " AND #{join_alias}.value <> ''" + + " AND #{join_alias}.id = (SELECT max(#{join_alias}_2.id) FROM #{CustomValue.table_name} #{join_alias}_2" + + " WHERE #{join_alias}_2.customized_type = #{join_alias}.customized_type" + + " AND #{join_alias}_2.customized_id = #{join_alias}.customized_id" + + " AND #{join_alias}_2.custom_field_id = #{join_alias}.custom_field_id)" + when 'string', 'text', 'list', 'date', 'bool' + "LEFT OUTER JOIN #{CustomValue.table_name} #{join_alias}" + + " ON #{join_alias}.customized_type = '#{self.class.customized_class.base_class.name}'" + + " AND #{join_alias}.customized_id = #{self.class.customized_class.table_name}.id" + + " AND #{join_alias}.custom_field_id = #{id}" + + " AND #{join_alias}.id = (SELECT max(#{join_alias}_2.id) FROM #{CustomValue.table_name} #{join_alias}_2" + + " WHERE #{join_alias}_2.customized_type = #{join_alias}.customized_type" + + " AND #{join_alias}_2.customized_id = #{join_alias}.customized_id" + + " AND #{join_alias}_2.custom_field_id = #{join_alias}.custom_field_id)" else nil end diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index 84eb3cf6a..8f7002ad0 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -253,7 +253,7 @@ class IssueQuery < Query if grouped? begin # Rails3 will raise an (unexpected) RecordNotFound if there's only a nil group value - r = Issue.visible.count(:group => group_by_statement, :include => [:status, :project], :conditions => statement) + r = Issue.visible.count(:joins => joins_for_order_statement(group_by_statement), :group => group_by_statement, :include => [:status, :project], :conditions => statement) rescue ActiveRecord::RecordNotFound r = {nil => issue_count} end diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 3b92ea4dd..693399362 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -877,9 +877,7 @@ class QueryTest < ActiveSupport::TestCase c = q.available_columns.find {|col| col.is_a?(QueryCustomFieldColumn) && col.custom_field.field_format == 'string' } assert c assert c.sortable - issues = Issue.includes([:assigned_to, :status, :tracker, :project, :priority]).where( - q.statement - ).order("#{c.sortable} ASC").all + issues = q.issues(:order => "#{c.sortable} ASC") values = issues.collect {|i| i.custom_value_for(c.custom_field).to_s} assert !values.empty? assert_equal values.sort, values @@ -890,9 +888,7 @@ class QueryTest < ActiveSupport::TestCase c = q.available_columns.find {|col| col.is_a?(QueryCustomFieldColumn) && col.custom_field.field_format == 'string' } assert c assert c.sortable - issues = Issue.includes([:assigned_to, :status, :tracker, :project, :priority]).where( - q.statement - ).order("#{c.sortable} DESC").all + issues = q.issues(:order => "#{c.sortable} DESC") values = issues.collect {|i| i.custom_value_for(c.custom_field).to_s} assert !values.empty? assert_equal values.sort.reverse, values @@ -903,9 +899,7 @@ class QueryTest < ActiveSupport::TestCase c = q.available_columns.find {|col| col.is_a?(QueryCustomFieldColumn) && col.custom_field.field_format == 'float' } assert c assert c.sortable - issues = Issue.includes([:assigned_to, :status, :tracker, :project, :priority]).where( - q.statement - ).order("#{c.sortable} ASC").all + issues = q.issues(:order => "#{c.sortable} ASC") values = issues.collect {|i| begin; Kernel.Float(i.custom_value_for(c.custom_field).to_s); rescue; nil; end}.compact assert !values.empty? assert_equal values.sort, values -- 2.39.5