Merge pull request #200 from mdsol/feature/optimize_accessible_operat…

…ion-sets [MCC-1182818] Optimize `operations_for_operation_sets`
mdsol · Mar 6, 2024 · 3ecfee5 · 3ecfee5
2 parents b3964ea + 4509817
commit 3ecfee5
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 19 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -3,11 +3,11 @@ language: ruby
 cache: bundler
 
 addons:
-  postgresql: "11"
+  postgresql: "12"
   apt:
     packages:
-    - postgresql-11
-    - postgresql-client-11
+    - postgresql-12
+    - postgresql-client-12
 
 env:
   global:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Changelog
 
+## 4.1.2
+* Updated PostgreSQL `operations_for_operation_sets` with performance improvements
+
 ## 4.1.1
 * Updated PostgreSQL `operations_for_operation_sets` to guarantee disable mergejoin
 

diff --git a/lib/policy_machine/version.rb b/lib/policy_machine/version.rb
@@ -1,3 +1,3 @@
 class PolicyMachine
-  VERSION = "4.1.1"
+  VERSION = "4.1.2"
 end
diff --git a/lib/policy_machine_storage_adapters/active_record/postgresql.rb b/lib/policy_machine_storage_adapters/active_record/postgresql.rb
@@ -7,10 +7,21 @@ class PolicyElement
       # representing all operations contained by the given operation set ids
       # optionally can give a list of operation names to filter by
       def self.operations_for_operation_sets(operation_set_ids, operation_names = nil)
+        query_args = [operation_set_ids]
+
+        operation_predicate =
+          if operation_names
+            query_args.append(operation_names)
+            'unique_identifier IN (?)'
+          else
+            query_args.append(PolicyMachineStorageAdapter::ActiveRecord.class_for_type('operation').name)
+            '"type" = ?'
+          end
+
         query = <<~SQL
           SET LOCAL enable_mergejoin TO FALSE;
 
-          WITH RECURSIVE accessible_operations AS (
+          WITH RECURSIVE accessible_operations AS MATERIALIZED (
             (
               SELECT
                 child_id,
@@ -28,23 +39,24 @@ def self.operations_for_operation_sets(operation_set_ids, operation_names = nil)
               FROM assignments
               JOIN accessible_operations ON accessible_operations.child_id = assignments.parent_id
             )
+          ), operations AS MATERIALIZED (
+            SELECT
+              id,
+              unique_identifier
+            FROM policy_elements
+            WHERE
+              id IN (SELECT child_id FROM accessible_operations)
+              AND #{operation_predicate}
           )
-          SELECT DISTINCT accessible_operations.operation_set_id, ops.unique_identifier
-          FROM accessible_operations
-          JOIN policy_elements ops ON ops.id = accessible_operations.child_id
-          WHERE ops.id IN (SELECT child_id FROM accessible_operations)
+          SELECT DISTINCT
+            ao.operation_set_id,
+            ops.unique_identifier
+          FROM operations ops
+          JOIN accessible_operations ao
+            on ao.child_id = ops.id
         SQL
 
-        sanitize_arg = [query, operation_set_ids]
-
-        if operation_names
-          query << "AND ops.unique_identifier IN (?);"
-          sanitize_arg << operation_names
-        else
-          type = PolicyMachineStorageAdapter::ActiveRecord.class_for_type('operation').name
-          query << "AND ops.type = '#{type}';"
-        end
-
+        sanitize_arg = [query] + query_args
         sanitized_query = sanitize_sql_for_assignment(sanitize_arg)
 
         # gives pairs of (opset_id, operation) representing all operations contained by an operation set