Revamp finding projects by namespaces
By using a JOIN we can remove the need for using 2 separate queries to find a project by its namespace. Combined with an index (only needed for PostgreSQL) this reduces the query time from ~245 ms (~520 ms for the first call) down to roughly 10 ms (~15 ms for the first call).
This commit is contained in:
parent
1190d0ab3d
commit
03417456f0
6 changed files with 78 additions and 22 deletions
|
@ -6,7 +6,7 @@ module CaseSensitivity
|
||||||
# Queries the given columns regardless of the casing used.
|
# Queries the given columns regardless of the casing used.
|
||||||
#
|
#
|
||||||
# Unlike other ActiveRecord methods this method only operates on a Hash.
|
# Unlike other ActiveRecord methods this method only operates on a Hash.
|
||||||
def case_insensitive_where(params)
|
def iwhere(params)
|
||||||
criteria = self
|
criteria = self
|
||||||
cast_lower = Gitlab::Database.postgresql?
|
cast_lower = Gitlab::Database.postgresql?
|
||||||
|
|
||||||
|
|
|
@ -40,6 +40,7 @@ class Project < ActiveRecord::Base
|
||||||
include Referable
|
include Referable
|
||||||
include Sortable
|
include Sortable
|
||||||
include AfterCommitQueue
|
include AfterCommitQueue
|
||||||
|
include CaseSensitivity
|
||||||
|
|
||||||
extend Gitlab::ConfigHelper
|
extend Gitlab::ConfigHelper
|
||||||
extend Enumerize
|
extend Enumerize
|
||||||
|
@ -235,13 +236,18 @@ class Project < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
|
|
||||||
def find_with_namespace(id)
|
def find_with_namespace(id)
|
||||||
return nil unless id.include?('/')
|
namespace_path, project_path = id.split('/')
|
||||||
|
|
||||||
id = id.split('/')
|
return nil if !namespace_path || !project_path
|
||||||
namespace = Namespace.by_path(id.first)
|
|
||||||
return nil unless namespace
|
|
||||||
|
|
||||||
where(namespace_id: namespace.id).where("LOWER(projects.path) = :path", path: id.second.downcase).first
|
# Use of unscoped ensures we're not secretly adding any ORDER BYs, which
|
||||||
|
# have a negative impact on performance (and aren't needed for this
|
||||||
|
# query).
|
||||||
|
unscoped.
|
||||||
|
joins(:namespace).
|
||||||
|
iwhere('namespaces.path' => namespace_path).
|
||||||
|
iwhere('projects.path' => project_path).
|
||||||
|
take
|
||||||
end
|
end
|
||||||
|
|
||||||
def visibility_levels
|
def visibility_levels
|
||||||
|
|
|
@ -0,0 +1,17 @@
|
||||||
|
class NamespacesProjectsPathLowerIndexes < ActiveRecord::Migration
|
||||||
|
disable_ddl_transaction!
|
||||||
|
|
||||||
|
def up
|
||||||
|
return unless Gitlab::Database.postgresql?
|
||||||
|
|
||||||
|
execute 'CREATE INDEX CONCURRENTLY index_on_namespaces_lower_path ON namespaces (LOWER(path));'
|
||||||
|
execute 'CREATE INDEX CONCURRENTLY index_on_projects_lower_path ON projects (LOWER(path));'
|
||||||
|
end
|
||||||
|
|
||||||
|
def down
|
||||||
|
return unless Gitlab::Database.postgresql?
|
||||||
|
|
||||||
|
remove_index :namespaces, name: :index_on_namespaces_lower_path
|
||||||
|
remove_index :projects, name: :index_on_projects_lower_path
|
||||||
|
end
|
||||||
|
end
|
|
@ -11,7 +11,7 @@
|
||||||
#
|
#
|
||||||
# It's strongly recommended that you check this file into your version control system.
|
# It's strongly recommended that you check this file into your version control system.
|
||||||
|
|
||||||
ActiveRecord::Schema.define(version: 20151005162154) do
|
ActiveRecord::Schema.define(version: 20151007120511) do
|
||||||
|
|
||||||
# These are extensions that must be enabled in order to support this database
|
# These are extensions that must be enabled in order to support this database
|
||||||
enable_extension "plpgsql"
|
enable_extension "plpgsql"
|
||||||
|
|
20
spec/benchmarks/models/project_spec.rb
Normal file
20
spec/benchmarks/models/project_spec.rb
Normal file
|
@ -0,0 +1,20 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe Project, benchmark: true do
|
||||||
|
describe '.find_with_namespace' do
|
||||||
|
let(:group) { create(:group, name: 'sisinmaru') }
|
||||||
|
let(:project) { create(:project, name: 'maru', namespace: group) }
|
||||||
|
|
||||||
|
describe 'using a capitalized namespace' do
|
||||||
|
benchmark_subject { described_class.find_with_namespace('sisinmaru/MARU') }
|
||||||
|
|
||||||
|
it { is_expected.to iterate_per_second(600) }
|
||||||
|
end
|
||||||
|
|
||||||
|
describe 'using a lowercased namespace' do
|
||||||
|
benchmark_subject { described_class.find_with_namespace('sisinmaru/maru') }
|
||||||
|
|
||||||
|
it { is_expected.to iterate_per_second(600) }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -1,11 +1,16 @@
|
||||||
require 'spec_helper'
|
require 'spec_helper'
|
||||||
|
|
||||||
describe CaseSensitivity do
|
describe CaseSensitivity do
|
||||||
describe '.case_insensitive_where' do
|
describe '.iwhere' do
|
||||||
let(:connection) { ActiveRecord::Base.connection }
|
let(:connection) { ActiveRecord::Base.connection }
|
||||||
let(:model) { Class.new { include CaseSensitivity } }
|
let(:model) { Class.new { include CaseSensitivity } }
|
||||||
|
|
||||||
describe 'using PostgreSQL' do
|
describe 'using PostgreSQL' do
|
||||||
|
before do
|
||||||
|
allow(Gitlab::Database).to receive(:postgresql?).and_return(true)
|
||||||
|
allow(Gitlab::Database).to receive(:mysql?).and_return(false)
|
||||||
|
end
|
||||||
|
|
||||||
describe 'with a single column/value pair' do
|
describe 'with a single column/value pair' do
|
||||||
it 'returns the criteria for a column and a value' do
|
it 'returns the criteria for a column and a value' do
|
||||||
criteria = double(:criteria)
|
criteria = double(:criteria)
|
||||||
|
@ -18,7 +23,7 @@ describe CaseSensitivity do
|
||||||
with(%q{LOWER("foo") = LOWER(:value)}, value: 'bar').
|
with(%q{LOWER("foo") = LOWER(:value)}, value: 'bar').
|
||||||
and_return(criteria)
|
and_return(criteria)
|
||||||
|
|
||||||
expect(model.case_insensitive_where(foo: 'bar')).to eq(criteria)
|
expect(model.iwhere(foo: 'bar')).to eq(criteria)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns the criteria for a column with a table, and a value' do
|
it 'returns the criteria for a column with a table, and a value' do
|
||||||
|
@ -32,7 +37,7 @@ describe CaseSensitivity do
|
||||||
with(%q{LOWER("foo"."bar") = LOWER(:value)}, value: 'bar').
|
with(%q{LOWER("foo"."bar") = LOWER(:value)}, value: 'bar').
|
||||||
and_return(criteria)
|
and_return(criteria)
|
||||||
|
|
||||||
expect(model.case_insensitive_where('foo.bar': 'bar')).to eq(criteria)
|
expect(model.iwhere(:'foo.bar' => 'bar')).to eq(criteria)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -57,7 +62,7 @@ describe CaseSensitivity do
|
||||||
with(%q{LOWER("bar") = LOWER(:value)}, value: 'baz').
|
with(%q{LOWER("bar") = LOWER(:value)}, value: 'baz').
|
||||||
and_return(final)
|
and_return(final)
|
||||||
|
|
||||||
got = model.case_insensitive_where(foo: 'bar', bar: 'baz')
|
got = model.iwhere(foo: 'bar', bar: 'baz')
|
||||||
|
|
||||||
expect(got).to eq(final)
|
expect(got).to eq(final)
|
||||||
end
|
end
|
||||||
|
@ -82,7 +87,8 @@ describe CaseSensitivity do
|
||||||
with(%q{LOWER("foo"."baz") = LOWER(:value)}, value: 'baz').
|
with(%q{LOWER("foo"."baz") = LOWER(:value)}, value: 'baz').
|
||||||
and_return(final)
|
and_return(final)
|
||||||
|
|
||||||
got = model.case_insensitive_where('foo.bar': 'bar', 'foo.baz': 'baz')
|
got = model.iwhere(:'foo.bar' => 'bar',
|
||||||
|
:'foo.baz' => 'baz')
|
||||||
|
|
||||||
expect(got).to eq(final)
|
expect(got).to eq(final)
|
||||||
end
|
end
|
||||||
|
@ -90,6 +96,11 @@ describe CaseSensitivity do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'using MySQL' do
|
describe 'using MySQL' do
|
||||||
|
before do
|
||||||
|
allow(Gitlab::Database).to receive(:postgresql?).and_return(false)
|
||||||
|
allow(Gitlab::Database).to receive(:mysql?).and_return(true)
|
||||||
|
end
|
||||||
|
|
||||||
describe 'with a single column/value pair' do
|
describe 'with a single column/value pair' do
|
||||||
it 'returns the criteria for a column and a value' do
|
it 'returns the criteria for a column and a value' do
|
||||||
criteria = double(:criteria)
|
criteria = double(:criteria)
|
||||||
|
@ -99,10 +110,10 @@ describe CaseSensitivity do
|
||||||
and_return('`foo`')
|
and_return('`foo`')
|
||||||
|
|
||||||
expect(model).to receive(:where).
|
expect(model).to receive(:where).
|
||||||
with(%q{LOWER(`foo`) = LOWER(:value)}, value: 'bar').
|
with(%q{`foo` = :value}, value: 'bar').
|
||||||
and_return(criteria)
|
and_return(criteria)
|
||||||
|
|
||||||
expect(model.case_insensitive_where(foo: 'bar')).to eq(criteria)
|
expect(model.iwhere(foo: 'bar')).to eq(criteria)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns the criteria for a column with a table, and a value' do
|
it 'returns the criteria for a column with a table, and a value' do
|
||||||
|
@ -113,10 +124,11 @@ describe CaseSensitivity do
|
||||||
and_return('`foo`.`bar`')
|
and_return('`foo`.`bar`')
|
||||||
|
|
||||||
expect(model).to receive(:where).
|
expect(model).to receive(:where).
|
||||||
with(%q{LOWER(`foo`.`bar`) = LOWER(:value)}, value: 'bar').
|
with(%q{`foo`.`bar` = :value}, value: 'bar').
|
||||||
and_return(criteria)
|
and_return(criteria)
|
||||||
|
|
||||||
expect(model.case_insensitive_where('foo.bar': 'bar')).to eq(criteria)
|
expect(model.iwhere(:'foo.bar' => 'bar')).
|
||||||
|
to eq(criteria)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -134,14 +146,14 @@ describe CaseSensitivity do
|
||||||
and_return('`bar`')
|
and_return('`bar`')
|
||||||
|
|
||||||
expect(model).to receive(:where).
|
expect(model).to receive(:where).
|
||||||
with(%q{LOWER(`foo`) = LOWER(:value)}, value: 'bar').
|
with(%q{`foo` = :value}, value: 'bar').
|
||||||
and_return(initial)
|
and_return(initial)
|
||||||
|
|
||||||
expect(initial).to receive(:where).
|
expect(initial).to receive(:where).
|
||||||
with(%q{LOWER(`bar`) = LOWER(:value)}, value: 'baz').
|
with(%q{`bar` = :value}, value: 'baz').
|
||||||
and_return(final)
|
and_return(final)
|
||||||
|
|
||||||
got = model.case_insensitive_where(foo: 'bar', bar: 'baz')
|
got = model.iwhere(foo: 'bar', bar: 'baz')
|
||||||
|
|
||||||
expect(got).to eq(final)
|
expect(got).to eq(final)
|
||||||
end
|
end
|
||||||
|
@ -159,14 +171,15 @@ describe CaseSensitivity do
|
||||||
and_return('`foo`.`baz`')
|
and_return('`foo`.`baz`')
|
||||||
|
|
||||||
expect(model).to receive(:where).
|
expect(model).to receive(:where).
|
||||||
with(%q{LOWER(`foo`.`bar`) = LOWER(:value)}, value: 'bar').
|
with(%q{`foo`.`bar` = :value}, value: 'bar').
|
||||||
and_return(initial)
|
and_return(initial)
|
||||||
|
|
||||||
expect(initial).to receive(:where).
|
expect(initial).to receive(:where).
|
||||||
with(%q{LOWER(`foo`.`baz`) = LOWER(:value)}, value: 'baz').
|
with(%q{`foo`.`baz` = :value}, value: 'baz').
|
||||||
and_return(final)
|
and_return(final)
|
||||||
|
|
||||||
got = model.case_insensitive_where('foo.bar': 'bar', 'foo.baz': 'baz')
|
got = model.iwhere(:'foo.bar' => 'bar',
|
||||||
|
:'foo.baz' => 'baz')
|
||||||
|
|
||||||
expect(got).to eq(final)
|
expect(got).to eq(final)
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue