Remove useless queries with false conditions (e.g 1=0)

6 files changed, 64 insertions, 0 deletions

diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb
index 85ae4985e58..c8a501d7319 100644
--- a/app/controllers/concerns/issuable_collections.rb
+++ b/app/controllers/concerns/issuable_collections.rb
@@ -15,6 +15,9 @@ module IssuableCollections
     # a new order into the collection.
     # We cannot use reorder to not mess up the paginated collection.
     issuable_ids = issuable_collection.map(&:id)
+
+    return {} if issuable_ids.empty?
+
     issuable_note_count = Note.count_for_collection(issuable_ids, @collection_type)
     issuable_votes_count = AwardEmoji.votes_for_collection(issuable_ids, @collection_type)
     issuable_merge_requests_count =
diff --git a/app/workers/process_commit_worker.rb b/app/workers/process_commit_worker.rb
index e9a5bd7f24e..2f7967cf531 100644
--- a/app/workers/process_commit_worker.rb
+++ b/app/workers/process_commit_worker.rb
@@ -53,6 +53,8 @@ class ProcessCommitWorker
   def update_issue_metrics(commit, author)
     mentioned_issues = commit.all_references(author).issues
 
+    return if mentioned_issues.empty?
+
     Issue::Metrics.where(issue_id: mentioned_issues.map(&:id), first_mentioned_in_commit_at: nil).
       update_all(first_mentioned_in_commit_at: commit.committed_date)
   end
diff --git a/changelogs/unreleased/29492-useless-queries.yml b/changelogs/unreleased/29492-useless-queries.yml
new file mode 100644
index 00000000000..266a04be352
--- /dev/null
+++ b/changelogs/unreleased/29492-useless-queries.yml
@@ -0,0 +1,4 @@
+---
+title: Remove useless queries with false conditions (e.g 1=0)
+merge_request: 10141
+author: mhasbini
diff --git a/spec/support/issuables_list_metadata_shared_examples.rb b/spec/support/issuables_list_metadata_shared_examples.rb
index 4c0f556e736..e1ac8fd60cd 100644
--- a/spec/support/issuables_list_metadata_shared_examples.rb
+++ b/spec/support/issuables_list_metadata_shared_examples.rb
@@ -33,4 +33,19 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil|
       expect(meta_data[id].upvotes).to eq(id + 2)
     end
   end
+
+  describe "when given empty collection" do
+    let(:project2) { create(:empty_project, :public) }
+
+    it "doesn't execute any queries with false conditions" do
+      get_action =
+        if action
+          proc { get action }
+        else
+          proc { get :index, namespace_id: project2.namespace, project_id: project2 }
+        end
+
+      expect(&get_action).not_to make_queries_matching(/WHERE (?:1=0|0=1)/)
+    end
+  end
 end
diff --git a/spec/support/matchers/query_matcher.rb b/spec/support/matchers/query_matcher.rb
new file mode 100644
index 00000000000..ac8c4ab91d9
--- /dev/null
+++ b/spec/support/matchers/query_matcher.rb
@@ -0,0 +1,33 @@
+RSpec::Matchers.define :make_queries_matching do |matcher, expected_count = nil|
+  supports_block_expectations
+
+  match do |block|
+    @counter = query_count(matcher, &block)
+    if expected_count
+      @counter.count == expected_count
+    else
+      @counter.count > 0
+    end
+  end
+
+  failure_message_when_negated do |_|
+    if expected_count
+      "expected #{matcher} not to match #{expected_count} queries, got #{@counter.count} matches:\n\n#{@counter.inspect}"
+    else
+      "expected #{matcher} not to match any query, got #{@counter.count} matches:\n\n#{@counter.inspect}"
+    end
+  end
+
+  failure_message do |_|
+    if expected_count
+      "expected #{matcher} to match #{expected_count} queries, got #{@counter.count} matches:\n\n#{@counter.inspect}"
+    else
+      "expected #{matcher} to match at least one query, got #{@counter.count} matches:\n\n#{@counter.inspect}"
+    end
+  end
+
+  def query_count(regex, &block)
+    @recorder = ActiveRecord::QueryRecorder.new(&block).log
+    @recorder.select{ |q| q.match(regex) }
+  end
+end
diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb
index 1c383d0514d..9afe2e610b9 100644
--- a/spec/workers/process_commit_worker_spec.rb
+++ b/spec/workers/process_commit_worker_spec.rb
@@ -99,6 +99,13 @@ describe ProcessCommitWorker do
 
       expect(metric.first_mentioned_in_commit_at).to eq(commit.committed_date)
     end
+
+    it "doesn't execute any queries with false conditions" do
+      allow(commit).to receive(:safe_message).
+        and_return("Lorem Ipsum")
+
+      expect { worker.update_issue_metrics(commit, user) }.not_to make_queries_matching(/WHERE (?:1=0|0=1)/)
+    end
   end
 
   describe '#build_commit' do