Commit 2c879643 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'revert-12499' into 'master'

Revert "Merge branch 'dm-drop-default-scope-on-sortable-finders' into 'master'"

Closes #34511

See merge request !12557
parents 3d49246d a3d7983b
......@@ -227,7 +227,7 @@ class Projects::IssuesController < Projects::ApplicationController
def issue
return @issue if defined?(@issue)
# The Sortable default scope causes performance issues when used with find_by
@noteable = @issue ||= @project.issues.find_by!(iid: params[:id])
@noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take!
return render_404 unless can?(current_user, :read_issue, @issue)
......
......@@ -5,25 +5,6 @@
module Sortable
extend ActiveSupport::Concern
module DropDefaultScopeOnFinders
# Override these methods to drop the `ORDER BY id DESC` default scope.
# See http://dba.stackexchange.com/a/110919 for why we do this.
%i[find find_by find_by!].each do |meth|
define_method meth do |*args, &block|
return super(*args, &block) if block
unordered_relation = unscope(:order)
# We cannot simply call `meth` on `unscope(:order)`, since that is also
# an instance of the same relation class this module is included into,
# which means we'd get infinite recursion.
# We explicitly use the original implementation to prevent this.
original_impl = method(__method__).super_method.unbind
original_impl.bind(unordered_relation).call(*args)
end
end
end
included do
# By default all models should be ordered
# by created_at field starting from newest
......@@ -37,10 +18,6 @@ module Sortable
scope :order_updated_asc, -> { reorder(updated_at: :asc) }
scope :order_name_asc, -> { reorder(name: :asc) }
scope :order_name_desc, -> { reorder(name: :desc) }
# All queries (relations) on this model are instances of this `relation_klass`.
relation_klass = relation_delegate_class(ActiveRecord::Relation)
relation_klass.prepend DropDefaultScopeOnFinders
end
module ClassMethods
......
......@@ -815,7 +815,7 @@ class Project < ActiveRecord::Base
end
def ci_service
@ci_service ||= ci_services.find_by(active: true)
@ci_service ||= ci_services.reorder(nil).find_by(active: true)
end
def deployment_services
......@@ -823,7 +823,7 @@ class Project < ActiveRecord::Base
end
def deployment_service
@deployment_service ||= deployment_services.find_by(active: true)
@deployment_service ||= deployment_services.reorder(nil).find_by(active: true)
end
def monitoring_services
......@@ -831,7 +831,7 @@ class Project < ActiveRecord::Base
end
def monitoring_service
@monitoring_service ||= monitoring_services.find_by(active: true)
@monitoring_service ||= monitoring_services.reorder(nil).find_by(active: true)
end
def jira_tracker?
......
---
title: Improve performance of lookups of issues, merge requests etc by dropping unnecessary ORDER BY clause
merge_request:
author:
require 'spec_helper'
describe Sortable do
let(:relation) { Issue.all }
describe '#where' do
it 'orders by id, descending' do
order_node = relation.where(iid: 1).order_values.first
expect(order_node).to be_a(Arel::Nodes::Descending)
expect(order_node.expr.name).to eq(:id)
end
end
describe '#find_by' do
it 'does not order' do
expect(relation).to receive(:unscope).with(:order).and_call_original
relation.find_by(iid: 1)
end
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