From 41bfe82b7a650f21b19a25204dde5a0eaf960d0f Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 15 Feb 2018 19:34:44 +0100 Subject: Optimise searching for users using short queries This optimises searching for users when using queries consisting out of one or two characters such as "ab". We optimise such cases by searching for `LOWER(name)` and `LOWER(username)` instead of using `ILIKE`. Using `LOWER` produces a _much_ better performing query. For example, when searching for all users matching the term "a" we'd produce the following plan: Limit (cost=637.69..637.74 rows=20 width=805) (actual time=41.983..41.995 rows=20 loops=1) Buffers: shared hit=8330 -> Sort (cost=637.69..638.61 rows=368 width=805) (actual time=41.982..41.990 rows=20 loops=1) Sort Key: (CASE WHEN ((name)::text = 'a'::text) THEN 0 WHEN ((username)::text = 'a'::text) THEN 1 WHEN ((email)::text = 'a'::text) THEN 2 ELSE 3 END), name Sort Method: top-N heapsort Memory: 35kB Buffers: shared hit=8330 -> Bitmap Heap Scan on users (cost=75.47..627.89 rows=368 width=805) (actual time=9.452..41.305 rows=277 loops=1) Recheck Cond: (((name)::text ~~* 'a'::text) OR ((username)::text ~~* 'a'::text) OR ((email)::text = 'a'::text)) Rows Removed by Index Recheck: 7601 Heap Blocks: exact=7636 Buffers: shared hit=8327 -> BitmapOr (cost=75.47..75.47 rows=368 width=0) (actual time=8.290..8.290 rows=0 loops=1) Buffers: shared hit=691 -> Bitmap Index Scan on index_users_on_name_trigram (cost=0.00..38.85 rows=180 width=0) (actual time=4.369..4.369 rows=4071 loops=1) Index Cond: ((name)::text ~~* 'a'::text) Buffers: shared hit=360 -> Bitmap Index Scan on index_users_on_username_trigram (cost=0.00..34.41 rows=188 width=0) (actual time=3.896..3.896 rows=4140 loops=1) Index Cond: ((username)::text ~~* 'a'::text) Buffers: shared hit=328 -> Bitmap Index Scan on users_email_key (cost=0.00..1.94 rows=1 width=0) (actual time=0.022..0.022 rows=0 loops=1) Index Cond: ((email)::text = 'a'::text) Buffers: shared hit=3 Planning time: 3.912 ms Execution time: 42.171 ms With the changes in this commit we now produce the following plan instead: Limit (cost=13257.48..13257.53 rows=20 width=805) (actual time=1.567..1.579 rows=20 loops=1) Buffers: shared hit=287 -> Sort (cost=13257.48..13280.93 rows=9379 width=805) (actual time=1.567..1.572 rows=20 loops=1) Sort Key: (CASE WHEN ((name)::text = 'a'::text) THEN 0 WHEN ((username)::text = 'a'::text) THEN 1 WHEN ((email)::text = 'a'::text) THEN 2 ELSE 3 END), name Sort Method: top-N heapsort Memory: 35kB Buffers: shared hit=287 -> Bitmap Heap Scan on users (cost=135.66..13007.91 rows=9379 width=805) (actual time=0.194..1.107 rows=277 loops=1) Recheck Cond: ((lower((name)::text) = 'a'::text) OR (lower((username)::text) = 'a'::text) OR ((email)::text = 'a'::text)) Heap Blocks: exact=277 Buffers: shared hit=287 -> BitmapOr (cost=135.66..135.66 rows=9379 width=0) (actual time=0.152..0.152 rows=0 loops=1) Buffers: shared hit=10 -> Bitmap Index Scan on yorick_test_users (cost=0.00..124.75 rows=9377 width=0) (actual time=0.101..0.101 rows=277 loops=1) Index Cond: (lower((name)::text) = 'a'::text) Buffers: shared hit=4 -> Bitmap Index Scan on index_on_users_lower_username (cost=0.00..1.94 rows=1 width=0) (actual time=0.035..0.035 rows=1 loops=1) Index Cond: (lower((username)::text) = 'a'::text) Buffers: shared hit=3 -> Bitmap Index Scan on users_email_key (cost=0.00..1.94 rows=1 width=0) (actual time=0.014..0.014 rows=0 loops=1) Index Cond: ((email)::text = 'a'::text) Buffers: shared hit=3 Planning time: 0.303 ms Execution time: 1.687 ms Here we can see the new query is 25 times faster compared to the old query. --- lib/gitlab/sql/pattern.rb | 16 ++++++++++++++-- lib/tasks/migrate/setup_postgresql.rake | 2 ++ 2 files changed, 16 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/sql/pattern.rb b/lib/gitlab/sql/pattern.rb index 5f0c98cb5a4..0e3a93aa236 100644 --- a/lib/gitlab/sql/pattern.rb +++ b/lib/gitlab/sql/pattern.rb @@ -25,7 +25,11 @@ module Gitlab query.length >= MIN_CHARS_FOR_PARTIAL_MATCHING end - def fuzzy_arel_match(column, query) + # column - The column name to search in. + # query - The text to search for. + # lower_exact_match - When set to `true` we'll fall back to using + # `LOWER(column) = query` instead of using `ILIKE`. + def fuzzy_arel_match(column, query, lower_exact_match: false) query = query.squish return nil unless query.present? @@ -34,9 +38,17 @@ module Gitlab if words.any? words.map { |word| arel_table[column].matches(to_pattern(word)) }.reduce(:and) else + sanitized_query = sanitize_sql_like(query) + # No words of at least 3 chars, but we can search for an exact # case insensitive match with the query as a whole - arel_table[column].matches(sanitize_sql_like(query)) + if lower_exact_match + Arel::Nodes::NamedFunction + .new('LOWER', [arel_table[column]]) + .eq(sanitized_query) + else + arel_table[column].matches(sanitized_query) + end end end diff --git a/lib/tasks/migrate/setup_postgresql.rake b/lib/tasks/migrate/setup_postgresql.rake index 31cbd651edb..1c7a8a90f5c 100644 --- a/lib/tasks/migrate/setup_postgresql.rake +++ b/lib/tasks/migrate/setup_postgresql.rake @@ -8,6 +8,7 @@ task setup_postgresql: :environment do require Rails.root.join('db/migrate/20170503185032_index_redirect_routes_path_for_like') require Rails.root.join('db/migrate/20171220191323_add_index_on_namespaces_lower_name.rb') require Rails.root.join('db/migrate/20180113220114_rework_redirect_routes_indexes.rb') + require Rails.root.join('db/migrate/20180215181245_users_name_lower_index.rb') NamespacesProjectsPathLowerIndexes.new.up AddUsersLowerUsernameEmailIndexes.new.up @@ -17,4 +18,5 @@ task setup_postgresql: :environment do IndexRedirectRoutesPathForLike.new.up AddIndexOnNamespacesLowerName.new.up ReworkRedirectRoutesIndexes.new.up + UsersNameLowerIndex.new.up end -- cgit v1.2.1 From 090eeb581b3809ab83d52f7baa2bcfbd63b1c2ba Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 15 Feb 2018 19:55:43 +0100 Subject: Added changelog for user search improvements --- lib/gitlab/sql/pattern.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/sql/pattern.rb b/lib/gitlab/sql/pattern.rb index 0e3a93aa236..53744bad1f4 100644 --- a/lib/gitlab/sql/pattern.rb +++ b/lib/gitlab/sql/pattern.rb @@ -38,16 +38,14 @@ module Gitlab if words.any? words.map { |word| arel_table[column].matches(to_pattern(word)) }.reduce(:and) else - sanitized_query = sanitize_sql_like(query) - # No words of at least 3 chars, but we can search for an exact # case insensitive match with the query as a whole if lower_exact_match Arel::Nodes::NamedFunction .new('LOWER', [arel_table[column]]) - .eq(sanitized_query) + .eq(query) else - arel_table[column].matches(sanitized_query) + arel_table[column].matches(sanitize_sql_like(query)) end end end -- cgit v1.2.1 From ec9cb6edba9c99241984506e3a098187f4cb1fbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=99=88=20=20jacopo=20beschi=20=F0=9F=99=89?= Date: Fri, 23 Feb 2018 14:23:09 +0000 Subject: Resolve "Milestone Quick Action not displayed with no project milestones but with group milestones" --- lib/gitlab/quick_actions/command_definition.rb | 15 ++++++--------- lib/gitlab/quick_actions/dsl.rb | 5 ++--- lib/gitlab/quick_actions/extractor.rb | 10 +++++----- 3 files changed, 13 insertions(+), 17 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/quick_actions/command_definition.rb b/lib/gitlab/quick_actions/command_definition.rb index 3937d9c153a..96415271316 100644 --- a/lib/gitlab/quick_actions/command_definition.rb +++ b/lib/gitlab/quick_actions/command_definition.rb @@ -24,15 +24,14 @@ module Gitlab action_block.nil? end - def available?(opts) + def available?(context) return true unless condition_block - context = OpenStruct.new(opts) context.instance_exec(&condition_block) end - def explain(context, opts, arg) - return unless available?(opts) + def explain(context, arg) + return unless available?(context) if explanation.respond_to?(:call) execute_block(explanation, context, arg) @@ -41,15 +40,13 @@ module Gitlab end end - def execute(context, opts, arg) - return if noop? || !available?(opts) + def execute(context, arg) + return if noop? || !available?(context) execute_block(action_block, context, arg) end - def to_h(opts) - context = OpenStruct.new(opts) - + def to_h(context) desc = description if desc.respond_to?(:call) desc = context.instance_exec(&desc) rescue '' diff --git a/lib/gitlab/quick_actions/dsl.rb b/lib/gitlab/quick_actions/dsl.rb index 536765305e1..d82dccd0db5 100644 --- a/lib/gitlab/quick_actions/dsl.rb +++ b/lib/gitlab/quick_actions/dsl.rb @@ -62,9 +62,8 @@ module Gitlab # Allows to define conditions that must be met in order for the command # to be returned by `.command_names` & `.command_definitions`. - # It accepts a block that will be evaluated with the context given to - # `CommandDefintion#to_h`. - # + # It accepts a block that will be evaluated with the context + # of a QuickActions::InterpretService instance # Example: # # condition do diff --git a/lib/gitlab/quick_actions/extractor.rb b/lib/gitlab/quick_actions/extractor.rb index c0878a34fb1..075ff91700c 100644 --- a/lib/gitlab/quick_actions/extractor.rb +++ b/lib/gitlab/quick_actions/extractor.rb @@ -29,7 +29,7 @@ module Gitlab # commands = extractor.extract_commands(msg) #=> [['labels', '~foo ~"bar baz"']] # msg #=> "hello\nworld" # ``` - def extract_commands(content, opts = {}) + def extract_commands(content) return [content, []] unless content content = content.dup @@ -37,7 +37,7 @@ module Gitlab commands = [] content.delete!("\r") - content.gsub!(commands_regex(opts)) do + content.gsub!(commands_regex) do if $~[:cmd] commands << [$~[:cmd], $~[:arg]].reject(&:blank?) '' @@ -60,8 +60,8 @@ module Gitlab # It looks something like: # # /^\/(?close|reopen|...)(?:( |$))(?[^\/\n]*)(?:\n|$)/ - def commands_regex(opts) - names = command_names(opts).map(&:to_s) + def commands_regex + names = command_names.map(&:to_s) @commands_regex ||= %r{ (? @@ -133,7 +133,7 @@ module Gitlab [content, commands] end - def command_names(opts) + def command_names command_definitions.flat_map do |command| next if command.noop? -- cgit v1.2.1