feat(matrix): attempt to optimise the query that determines the integ…

…rations
pact-foundation · Dec 30, 2019 · afde01e · afde01e
1 parent fff3805
commit afde01e
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 16 deletions.
diff --git a/lib/pact_broker/matrix/query_builder.rb b/lib/pact_broker/matrix/query_builder.rb
@@ -1,7 +1,7 @@
 module PactBroker
   module Matrix
     class QueryBuilder
-      def self.provider_or_provider_version_or_verification_in(selectors, allow_null_provider_version = false, qualifier)
+      def self.provider_or_provider_version_matches_selectors(selectors, allow_null_provider_version = false, qualifier)
         most_specific_criteria = selectors.collect(&:most_specific_provider_criterion)
 
         provider_version_ids = collect_ids(most_specific_criteria, :pacticipant_version_id)

diff --git a/lib/pact_broker/matrix/quick_row.rb b/lib/pact_broker/matrix/quick_row.rb
@@ -38,6 +38,14 @@ class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consum
       ALL_COLUMNS = CONSUMER_COLUMNS + CONSUMER_VERSION_COLUMNS + PROVIDER_COLUMNS + PROVIDER_VERSION_COLUMNS
 
       SELECT_ALL_COLUMN_ARGS = [:select_all_columns] + ALL_COLUMNS
+      PACTICIPANT_NAMES_AND_IDS = [
+        Sequel[:lp][:consumer_id], Sequel[:consumers][:name].as(:consumer_name),
+        Sequel[:lp][:provider_id], Sequel[:providers][:name].as(:provider_name)
+      ]
+      PACTICIPANT_VERSION_IDS = [
+        Sequel[:lp][:consumer_version_id],
+        Sequel[:lv][:provider_version_id]
+      ]
 
       associate(:many_to_one, :pact_publication, :class => "PactBroker::Pacts::PactPublication", :key => :pact_publication_id, :primary_key => :id)
       associate(:many_to_one, :provider, :class => "PactBroker::Domain::Pacticipant", :key => :provider_id, :primary_key => :id)
@@ -54,6 +62,15 @@ class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consum
 
         select *SELECT_ALL_COLUMN_ARGS
 
+        def distinct_integrations selectors
+          select(*(PACTICIPANT_NAMES_AND_IDS + PACTICIPANT_VERSION_IDS))
+            .distinct
+            .matching_selectors(selectors)
+            .from_self
+            .select(:consumer_name, :consumer_id, :provider_name, :provider_id)
+            .distinct
+        end
+
         def matching_selectors selectors
           if selectors.size == 1
             matching_one_selector(selectors.first)
@@ -65,8 +82,7 @@ def matching_selectors selectors
         # When we have one selector, we need to join ALL the verifications to find out
         # what integrations exist
         def matching_one_selector(selector)
-          select_all_columns
-            .join_verifications
+          join_verifications
             .join_pacticipants_and_pacticipant_versions
             .where {
               QueryBuilder.consumer_or_consumer_version_or_provider_or_provider_or_provider_version_match_selector(selector)
@@ -83,8 +99,7 @@ def matching_one_selector(selector)
         # and THEN join them to the pacts, so that we get a row for the pact with null provider version
         # and verification fields.
         def matching_multiple_selectors(selectors)
-          select_all_columns
-            .join_verifications_for(selectors)
+          join_verifications_for(selectors)
             .join_pacticipants_and_pacticipant_versions
             .where {
               Sequel.&(
@@ -94,17 +109,17 @@ def matching_multiple_selectors(selectors)
             }
             .from_self(alias: :t9)
             .where {
-              QueryBuilder.provider_or_provider_version_or_verification_in(selectors, true, :t9)
+              QueryBuilder.provider_or_provider_version_matches_selectors(selectors, true, :t9)
             }
         end
 
         def verifications_for(selectors)
           db[LV]
-            .select(:verification_id, :provider_version_id, :pact_version_id)
+            .select(:verification_id, :provider_version_id, :pact_version_id, :provider_id)
             .where {
               Sequel.&(
                 QueryBuilder.consumer_in_pacticipant_ids(selectors),
-                QueryBuilder.provider_or_provider_version_or_verification_in(selectors, false, LV)
+                QueryBuilder.provider_or_provider_version_matches_selectors(selectors, false, LV)
               )
             }
         end

diff --git a/lib/pact_broker/matrix/repository.rb b/lib/pact_broker/matrix/repository.rb
@@ -81,14 +81,8 @@ def find_compatible_pacticipant_versions selectors
       # If two or more are specified, just return the integrations that involve the specified pacticipants
       def find_integrations_for_specified_selectors(resolved_specified_selectors)
         specified_pacticipant_names = resolved_specified_selectors.collect(&:pacticipant_name)
-
-        # Must do select after matching selectors so we only select the columns we actually want
-        # Otherwise we end up with a really long query!
         QuickRow
-          .matching_selectors(resolved_specified_selectors)
-          .from_self
-          .select(:consumer_name, :consumer_id, :provider_name, :provider_id)
-          .distinct
+          .distinct_integrations(resolved_specified_selectors)
           .collect(&:to_hash)
           .collect do | hash |
             required = is_a_row_for_this_integration_required?(specified_pacticipant_names, hash[:consumer_name])
@@ -124,7 +118,7 @@ def apply_latestby options, selectors, lines
       end
 
       def query_matrix selectors, options
-        query = options[:latestby] ? QuickRow.eager_all_the_things : Row.select_all
+        query = options[:latestby] ? QuickRow.select_all_columns.eager_all_the_things : Row.select_all
         query = query.matching_selectors(selectors)
         query = query.limit(options[:limit]) if options[:limit]
         query