Added FromUnion to easily select from a UNION

This commit adds the module `FromUnion`, which provides the class method
`from_union`. This simplifies the process of selecting data from the
result of a UNION, and reduces the likelihood of making mistakes. As a
result, instead of this:

    union = Gitlab::SQL::Union.new([foo, bar])

    Foo.from("(#{union.to_sql}) #{Foo.table_name}")

We can now write this instead:

    Foo.from_union([foo, bar])

This commit also includes some changes to make this new setup work
properly. For example, a bug in Rails 4
(https://github.com/rails/rails/issues/24193) would break the use of
`from("sub-query-here").includes(:relation)` in certain cases. There was
also a CI query which appeared to repeat a lot of conditions from an
outer query on an inner query, which isn't necessary.

Finally, we include a RuboCop cop to ensure developers use this new
module, instead of using Gitlab::SQL::Union directly.

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/51307

1 files changed, 6 insertions, 9 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index e19bf62dcd0..dd5d494997d 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -14,6 +14,7 @@ class MergeRequest < ActiveRecord::Base
   include Gitlab::Utils::StrongMemoize
   include LabelEventable
   include ReactiveCaching
+  include FromUnion
 
   self.reactive_cache_key = ->(model) { [model.project.id, model.iid] }
   self.reactive_cache_refresh_interval = 10.minutes
@@ -237,11 +238,10 @@ class MergeRequest < ActiveRecord::Base
   def self.in_projects(relation)
     # unscoping unnecessary conditions that'll be applied
     # when executing `where("merge_requests.id IN (#{union.to_sql})")`
-    source = unscoped.where(source_project_id: relation).select(:id)
-    target = unscoped.where(target_project_id: relation).select(:id)
-    union  = Gitlab::SQL::Union.new([source, target])
+    source = unscoped.where(source_project_id: relation)
+    target = unscoped.where(target_project_id: relation)
 
-    where("merge_requests.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
+    from_union([source, target])
   end
 
   # This is used after project import, to reset the IDs to the correct
@@ -740,11 +740,8 @@ class MergeRequest < ActiveRecord::Base
     # compared to using OR statements. We're using UNION ALL since the queries
     # used won't produce any duplicates (e.g. a note for a commit can't also be
     # a note for an MR).
-    union = Gitlab::SQL::Union
-      .new([notes, commit_notes], remove_duplicates: false)
-      .to_sql
-
-    Note.from("(#{union}) #{Note.table_name}")
+    Note
+      .from_union([notes, commit_notes], remove_duplicates: false)
       .includes(:noteable)
   end