From 07d658ab821202e2e2ecee197a5f6e575ff7ae7e Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Tue, 11 Apr 2017 14:55:44 +0200 Subject: [PATCH] Improve speed of search [SCI-1157] --- app/helpers/database_helper.rb | 8 ++ app/models/asset.rb | 4 +- app/models/checklist.rb | 2 +- app/models/comment.rb | 10 +- app/models/concerns/searchable_model.rb | 20 ++- app/models/experiment.rb | 2 +- app/models/my_module.rb | 4 +- app/models/my_module_group.rb | 2 +- app/models/protocol.rb | 4 +- app/models/report.rb | 2 +- app/models/result.rb | 2 +- app/models/sample.rb | 46 +++++-- app/models/sample_custom_field.rb | 2 + app/models/sample_group.rb | 2 + app/models/sample_type.rb | 2 + app/models/step.rb | 2 +- app/models/table.rb | 6 +- app/models/tag.rb | 2 +- .../20170407082423_add_team_id_to_steps.rb | 114 ++++++++++++++++++ 19 files changed, 190 insertions(+), 46 deletions(-) create mode 100644 db/migrate/20170407082423_add_team_id_to_steps.rb diff --git a/app/helpers/database_helper.rb b/app/helpers/database_helper.rb index f84089728..d9b325bb1 100644 --- a/app/helpers/database_helper.rb +++ b/app/helpers/database_helper.rb @@ -26,6 +26,14 @@ module DatabaseHelper ) end + # Create gin trigram index with filtered out html tags. PostgreSQL only! + def add_gin_index_without_tags(table, column) + ActiveRecord::Base.connection.execute( + "CREATE INDEX index_#{table}_on_#{column} ON " \ + "#{table} USING gin ((trim_html_tags(#{column})) gin_trgm_ops);" + ) + end + # Get size of whole table & its indexes # (in bytes). PostgreSQL only! def get_table_size(table) diff --git a/app/models/asset.rb b/app/models/asset.rb index f31358cef..a7bddfc41 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -96,15 +96,15 @@ class Asset < ActiveRecord::Base Step .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) .joins(:step_assets) - .select("step_assets.id") .distinct + .pluck('step_assets.id') result_ids = Result .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) .joins(:result_asset) - .select("result_assets.id") .distinct + .pluck('result_assets.id') if query a_query = query.strip diff --git a/app/models/checklist.rb b/app/models/checklist.rb index 54206b1fb..738761c80 100644 --- a/app/models/checklist.rb +++ b/app/models/checklist.rb @@ -26,7 +26,7 @@ class Checklist < ActiveRecord::Base step_ids = Step .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select("id") + .pluck(:id) if query a_query = query.strip diff --git a/app/models/comment.rb b/app/models/comment.rb index 595e4f707..8515e18ce 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -20,19 +20,19 @@ class Comment < ActiveRecord::Base project_ids = Project .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select(:id) + .pluck(:id) my_module_ids = MyModule .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select(:id) + .pluck(:id) step_ids = Step .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select(:id) + .pluck(:id) result_ids = Result .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select(:id) + .pluck(:id) if query a_query = query.strip @@ -57,7 +57,7 @@ class Comment < ActiveRecord::Base step_ids, 'StepComment', result_ids, 'ResultComment' ) - .where_attributes_like([:message, 'users.full_name'], a_query) + .where_attributes_like(['message', 'users.full_name'], a_query) # Show all results if needed if page == Constants::SEARCH_NO_LIMIT diff --git a/app/models/concerns/searchable_model.rb b/app/models/concerns/searchable_model.rb index cae0f5837..54c5b32fa 100644 --- a/app/models/concerns/searchable_model.rb +++ b/app/models/concerns/searchable_model.rb @@ -22,19 +22,13 @@ module SearchableModel if query.is_a? Array - rich_text_regex = '<*strong>|<*href>|<*div>|' \ - '<*link>|<*span>|<%class%>|<%href%>|' \ - '<%data%>|<*sub>|<*sup>|<*blockquote>|<*li>|' \ - '<%style%>|<*ol>|<*ul>|<*pre>' - - if attrs.length > 0 + unless attrs.empty? where_str = (attrs.map.with_index do |a, i| - "REGEXP_REPLACE(#{a}, E'#{rich_text_regex}','', 'g')" \ - "ILIKE ANY (array[ :t#{i}]) OR " + "(trim_html_tags(#{a})) ILIKE ANY (array[ :t#{i}]) OR " end ).join[0..-5] - vals = (attrs.map.with_index do |a, i| + vals = (attrs.map.with_index do |_, i| ["t#{i}".to_sym, query] end ).to_h @@ -42,14 +36,14 @@ module SearchableModel return where(where_str, vals) end else - if attrs.length > 0 + unless attrs.empty? where_str = (attrs.map.with_index do |a, i| - "REGEXP_REPLACE(#{a}, E'#{rich_text_regex}'," \ - " '', 'g' ) ILIKE :t#{i} OR " + "(trim_html_tags(#{a})) ILIKE :t#{i} OR " end ).join[0..-5] - vals = (attrs.map.with_index { |a,i| [ "t#{i}".to_sym, "%#{query}%" ] }).to_h + vals = (attrs.map.with_index { |_, i| ["t#{i}".to_sym, query.to_s] }) + .to_h return where(where_str, vals) end diff --git a/app/models/experiment.rb b/app/models/experiment.rb index 67f4053cf..07f137273 100644 --- a/app/models/experiment.rb +++ b/app/models/experiment.rb @@ -43,7 +43,7 @@ class Experiment < ActiveRecord::Base project_ids = Project .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select('id') + .pluck(:id) if query a_query = '%' + query.strip.gsub('_', '\\_').gsub('%', '\\%') + '%' diff --git a/app/models/my_module.rb b/app/models/my_module.rb index a2e0d6ebf..ffb2caa32 100644 --- a/app/models/my_module.rb +++ b/app/models/my_module.rb @@ -52,7 +52,7 @@ class MyModule < ActiveRecord::Base exp_ids = Experiment .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select("id") + .pluck(:id) if query a_query = '%' + query.strip.gsub('_', '\\_').gsub('%', '\\%') + '%' @@ -71,7 +71,7 @@ class MyModule < ActiveRecord::Base new_query = MyModule .distinct .where('my_modules.experiment_id IN (?)', experiments_ids) - .where_attributes_like([:name], a_query) + .where_attributes_like([:name, :description], a_query) if include_archived return new_query diff --git a/app/models/my_module_group.rb b/app/models/my_module_group.rb index 0eb76a239..9efa6b368 100644 --- a/app/models/my_module_group.rb +++ b/app/models/my_module_group.rb @@ -15,7 +15,7 @@ class MyModuleGroup < ActiveRecord::Base exp_ids = Experiment .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select("id") + .pluck(:id) if query diff --git a/app/models/protocol.rb b/app/models/protocol.rb index 79efff5ad..5b1d98bf7 100644 --- a/app/models/protocol.rb +++ b/app/models/protocol.rb @@ -109,14 +109,14 @@ class Protocol < ActiveRecord::Base def self.search(user, include_archived, query = nil, page = 1) team_ids = Team.joins(:user_teams) .where('user_teams.user_id = ?', user.id) - .select('id') .distinct + .pluck(:id) module_ids = MyModule.search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select('id') + .pluck(:id) where_str = '(protocol_type IN (?) AND my_module_id IN (?)) OR ' \ diff --git a/app/models/report.rb b/app/models/report.rb index 5d1b5e599..dc2e4da7a 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -28,7 +28,7 @@ class Report < ActiveRecord::Base project_ids = Project .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select("id") + .pluck(:id) if query a_query = query.strip diff --git a/app/models/result.rb b/app/models/result.rb index 46faa38a7..a962e0afd 100644 --- a/app/models/result.rb +++ b/app/models/result.rb @@ -33,7 +33,7 @@ class Result < ActiveRecord::Base module_ids = MyModule .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select("id") + .pluck(:id) if query a_query = query.strip diff --git a/app/models/sample.rb b/app/models/sample.rb index b0d26b907..fcab9bcdc 100644 --- a/app/models/sample.rb +++ b/app/models/sample.rb @@ -26,8 +26,8 @@ class Sample < ActiveRecord::Base ) team_ids = Team.joins(:user_teams) .where('user_teams.user_id = ?', user.id) - .select('id') .distinct + .pluck(:id) if query a_query = '%' + query.strip.gsub('_', '\\_').gsub('%', '\\%') + '%' @@ -43,6 +43,33 @@ class Sample < ActiveRecord::Base return new_query else + user_ids = User + .joins(:user_teams) + .where('user_teams.team_id IN (?)', team_ids) + .where_attributes_like(['users.full_name'], a_query) + .pluck(:id) + + sample_ids = Sample + .joins(:user) + .where('team_id IN (?)', team_ids) + .where_attributes_like(['name'], a_query) + .pluck(:id) + + sample_type_ids = SampleType + .where('team_id IN (?)', team_ids) + .where_attributes_like(['name'], a_query) + .pluck(:id) + + sample_group_ids = SampleGroup + .where('team_id IN (?)', team_ids) + .where_attributes_like(['name'], a_query) + .pluck(:id) + + sample_custom_fields = SampleCustomField + .joins(:sample) + .where('samples.team_id IN (?)', team_ids) + .where_attributes_like(['value'], a_query) + .pluck(:id) new_query = Sample .distinct .joins(:user) @@ -52,17 +79,12 @@ class Sample < ActiveRecord::Base 'samples.sample_group_id = sample_groups.id') .joins('LEFT OUTER JOIN sample_custom_fields ON ' \ 'samples.id = sample_custom_fields.sample_id') - .where('samples.team_id IN (?)', team_ids) - .where_attributes_like( - [ - 'samples.name', - 'sample_types.name', - 'sample_groups.name', - 'users.full_name', - 'sample_custom_fields.value' - ], - a_query - ) + .where('samples.user_id IN (?) OR samples.id IN (?) ' \ + 'OR sample_types.id IN (?) ' \ + 'OR sample_groups.id IN (?)' \ + 'OR sample_custom_fields.id IN (?)', + user_ids, sample_ids, sample_type_ids, + sample_group_ids, sample_custom_fields) end # Show all results if needed if page == Constants::SEARCH_NO_LIMIT diff --git a/app/models/sample_custom_field.rb b/app/models/sample_custom_field.rb index 043bda051..95d1989a8 100644 --- a/app/models/sample_custom_field.rb +++ b/app/models/sample_custom_field.rb @@ -1,4 +1,6 @@ class SampleCustomField < ActiveRecord::Base + include SearchableModel + auto_strip_attributes :value, nullify: false validates :value, presence: true, diff --git a/app/models/sample_group.rb b/app/models/sample_group.rb index e1465b667..0e647012c 100644 --- a/app/models/sample_group.rb +++ b/app/models/sample_group.rb @@ -1,4 +1,6 @@ class SampleGroup < ActiveRecord::Base + include SearchableModel + auto_strip_attributes :name, :color, nullify: false validates :name, presence: true, diff --git a/app/models/sample_type.rb b/app/models/sample_type.rb index d05ed9d3a..9cf1f883f 100644 --- a/app/models/sample_type.rb +++ b/app/models/sample_type.rb @@ -1,4 +1,6 @@ class SampleType < ActiveRecord::Base + include SearchableModel + auto_strip_attributes :name, nullify: false validates :name, presence: true, diff --git a/app/models/step.rb b/app/models/step.rb index 38e01a9cc..5624b8755 100644 --- a/app/models/step.rb +++ b/app/models/step.rb @@ -45,7 +45,7 @@ class Step < ActiveRecord::Base protocol_ids = Protocol .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select("id") + .pluck(:id) if query a_query = query.strip diff --git a/app/models/table.rb b/app/models/table.rb index 45ed3252e..27f1e694a 100644 --- a/app/models/table.rb +++ b/app/models/table.rb @@ -26,15 +26,15 @@ class Table < ActiveRecord::Base Step .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) .joins(:step_tables) - .select("step_tables.id") .distinct + .pluck('step_tables.id') result_ids = Result .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) .joins(:result_table) - .select("result_tables.id") .distinct + .pluck('result_tables.id') if query a_query = query.strip @@ -66,7 +66,7 @@ class Table < ActiveRecord::Base .joins("LEFT OUTER JOIN results ON result_tables.result_id = results.id") .where("step_tables.id IN (?) OR result_tables.id IN (?)", step_ids, result_ids) .where( - '(tables.name ILIKE ANY (array[?])'\ + '(trim_html_tags(tables.name) ILIKE ANY (array[?])'\ 'OR tables.data_vector @@ to_tsquery(?))', a_query, s_query diff --git a/app/models/tag.rb b/app/models/tag.rb index 93509cdc2..0c93b4209 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -20,7 +20,7 @@ class Tag < ActiveRecord::Base project_ids = Project .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .select("id") + .pluck(:id) if query a_query = query.strip diff --git a/db/migrate/20170407082423_add_team_id_to_steps.rb b/db/migrate/20170407082423_add_team_id_to_steps.rb new file mode 100644 index 000000000..c070a78f3 --- /dev/null +++ b/db/migrate/20170407082423_add_team_id_to_steps.rb @@ -0,0 +1,114 @@ +require File.expand_path('app/helpers/database_helper') +include DatabaseHelper + +class AddTeamIdToSteps < ActiveRecord::Migration + def up + if db_adapter_is? 'PostgreSQL' + # Removing old indexes + remove_index :projects, :name if index_exists?(:projects, :name) + remove_index :my_modules, :name if index_exists?(:my_modules, :name) + remove_index :my_modules, :description if index_exists?(:my_modules, :description) + remove_index :protocols, :name if index_exists?(:protocols, :name) + remove_index :protocols, :description if index_exists?(:protocols, :description) + remove_index :protocols, :authors if index_exists?(:protocols, :authors) + remove_index :protocol_keywords, :name if index_exists?(:protocol_keywords, :name) + remove_index :my_module_groups, :name if index_exists?(:my_module_groups, :name) + remove_index :tags, :name if index_exists?(:tags, :name) + remove_index :results, :name if index_exists?(:results, :name) + remove_index :result_texts, :text if index_exists?(:result_texts, :text) + remove_index :reports, :name if index_exists?(:reports, :name) + remove_index :reports, :description if index_exists?(:reports, :description) + remove_index :assets, :file_file_name if index_exists?(:assets, :file_file_name) + remove_index :samples, :name if index_exists?(:samples, :name) + remove_index :sample_types, :name if index_exists?(:sample_types, :name) + remove_index :sample_groups, :name if index_exists?(:sample_groups, :name) + remove_index :sample_custom_fields, :value if index_exists?(:sample_custom_fields, :value) + remove_index :steps, :name if index_exists?(:steps, :name) + remove_index :steps, :description if index_exists?(:steps, :description) + remove_index :checklists, :name if index_exists?(:checklists, :name) + remove_index :checklist_items, :text if index_exists?(:checklist_items, :text) + remove_index :tables, :name if index_exists?(:tables, :name) + remove_index :users, :full_name if index_exists?(:users, :full_name) + remove_index :comments, :message if index_exists?(:comments, :message) + remove_index :comments, :type if index_exists?(:comments, :type) + remove_index :protocols, :protocol_type if index_exists?(:protocols, :protocol_type) + remove_index :checklists, :step_id if index_exists?(:checklists, :step_id) + + execute( + "CREATE OR REPLACE FUNCTION trim_html_tags(IN input TEXT, OUT output TEXT) AS $$ + SELECT regexp_replace(input, + E'<.*?>|\\\\[#.*\\\\]|\\\\[@.*\\\\]', + '', + 'g'); + $$ LANGUAGE SQL;" + ) + + add_gin_index_without_tags :projects, :name + add_gin_index_without_tags :my_modules, :name + add_gin_index_without_tags :my_module_groups, :name + add_gin_index_without_tags :my_modules, :description + add_gin_index_without_tags :protocols, :name + add_gin_index_without_tags :protocols, :description + add_gin_index_without_tags :protocols, :authors + add_gin_index_without_tags :protocol_keywords, :name + add_gin_index_without_tags :tags, :name + add_gin_index_without_tags :results, :name + add_gin_index_without_tags :result_texts, :text + add_gin_index_without_tags :reports, :name + add_gin_index_without_tags :reports, :description + add_gist_index :assets, :file_file_name + add_gin_index_without_tags :samples, :name + add_gin_index_without_tags :sample_types, :name + add_gin_index_without_tags :sample_groups, :name + add_gin_index_without_tags :sample_custom_fields, :value + add_gin_index_without_tags :steps, :name + add_gin_index_without_tags :steps, :description + add_gin_index_without_tags :checklists, :name + add_gin_index_without_tags :checklist_items, :text + add_gin_index_without_tags :tables, :name + add_gin_index_without_tags :users, :full_name + add_gin_index_without_tags :comments, :message + add_index :comments, :type + add_index :protocols, :protocol_type + add_index :checklists, :step_id + end + end + + def down + # remove_index :steps, :team_id + # remove_column :steps, :team_id, :integer + + if db_adapter_is? 'PostgreSQL' + remove_index :projects, :name + remove_index :my_modules, :name + remove_index :my_modules, :description + remove_index :my_module_groups, :name + remove_index :protocols, :name + remove_index :protocols, :description + remove_index :protocols, :authors + remove_index :protocol_keywords, :name + remove_index :tags, :name + remove_index :results, :name + remove_index :result_texts, :text + remove_index :reports, :name + remove_index :reports, :description + remove_index :assets, :file_file_name + remove_index :samples, :name + remove_index :sample_types, :name + remove_index :sample_groups, :name + remove_index :sample_custom_fields, :value + remove_index :steps, :name + remove_index :steps, :description + remove_index :checklists, :name + remove_index :checklist_items, :text + remove_index :tables, :name + remove_index :users, :full_name + remove_index :comments, :message + remove_index :comments, :type + remove_index :protocols, :protocol_type + remove_index :checklists, :step_id + + execute('DROP FUNCTION IF EXISTS trim_html_tags(IN input TEXT);') + end + end +end