Commit c8cafb22 authored by Andreas Brandl's avatar Andreas Brandl

Allow choice between #where or #from.

Immediately using #from here requires a lot of changes in
other finders (e.g. IssuableFinder, TodosFinder). In all places where we
use #merge, this goes completely the wrong way when passed in a relation
that was built with `#from(...)`: The original query's FROM part gets
completely replaced.

This avoids changing all other places and focuses on improving
SnippetFinder with the downside of two (small) codepaths to do the same
thing.
parent 78d84c13
...@@ -56,7 +56,7 @@ class SnippetsFinder < UnionFinder ...@@ -56,7 +56,7 @@ class SnippetsFinder < UnionFinder
end end
def feature_available_projects def feature_available_projects
projects = Project.public_or_visible_to_user(current_user) do |part| projects = Project.public_or_visible_to_user(current_user, use_conditions_only: false) do |part|
part.with_feature_available_for_user(:snippets, current_user) part.with_feature_available_for_user(:snippets, current_user)
end.select(:id) end.select(:id)
......
...@@ -321,7 +321,11 @@ class Project < ActiveRecord::Base ...@@ -321,7 +321,11 @@ class Project < ActiveRecord::Base
# the query, e.g. to apply .with_feature_available_for_user on top of it. # the query, e.g. to apply .with_feature_available_for_user on top of it.
# This is useful for performance as we can stick those additional filters # This is useful for performance as we can stick those additional filters
# at the bottom of e.g. the UNION. # at the bottom of e.g. the UNION.
def self.public_or_visible_to_user(user = nil, &block) #
# Optionally, turning `use_conditions_only` off leads to returning a
# relation using #from instead of #where. This can perform much better
# but leads to trouble when used in conjunction with AR's #merge method.
def self.public_or_visible_to_user(user = nil, use_conditions_only: true, &block)
# If we don't get a block passed, use identity to avoid if/else repetitions # If we don't get a block passed, use identity to avoid if/else repetitions
block = ->(part) { part } unless block_given? block = ->(part) { part } unless block_given?
...@@ -345,8 +349,12 @@ class Project < ActiveRecord::Base ...@@ -345,8 +349,12 @@ class Project < ActiveRecord::Base
# We use a UNION here instead of OR clauses since this results in better # We use a UNION here instead of OR clauses since this results in better
# performance. # performance.
union = Gitlab::SQL::Union.new([authorized_projects.select('projects.id'), visible_projects.select('projects.id')]) union = Gitlab::SQL::Union.new([authorized_projects.select('projects.id'), visible_projects.select('projects.id')])
# TODO: from("(#{union.to_sql}) AS #{table_name}")
where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection if use_conditions_only
where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
else
from("(#{union.to_sql}) AS #{table_name}")
end
else else
block.call(public_to_user) block.call(public_to_user)
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment