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.

2 files changed, 12 insertions, 4 deletions

diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb
index c4a84d88dbd..581cd5f47a7 100644
--- a/app/finders/snippets_finder.rb
+++ b/app/finders/snippets_finder.rb
@@ -56,7 +56,7 @@ class SnippetsFinder < UnionFinder
   end
 
   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)
     end.select(:id)
 
diff --git a/app/models/project.rb b/app/models/project.rb
index 5c6088d120f..a72e3388af8 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -321,7 +321,11 @@ class Project < ActiveRecord::Base
   # 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
   # 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
     block = ->(part) { part } unless block_given?
 
@@ -345,8 +349,12 @@ class Project < ActiveRecord::Base
       # We use a UNION here instead of OR clauses since this results in better
       # performance.
       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
       block.call(public_to_user)
     end