rbac: introduce use_sql_view

For queries with virtual_attributes, it can be slow to embed the
attributes in the primary query because it runs the attribute's
sub query for every row in the base table.

So even though you are using limit(20), it runs the virtual attribute
subquery for every row in the base table (think 6k times)

This is only really an issue on the services page, but other larger
tables like vms can benefit from this optimization.

This option embeds the table query in an inline view
think `select * from (select * from table) as inline_view`

This is opt-in behavior, and only the Services pages will use this
optimization for now.

app/models/miq_report/generator.rb

@@ -319,6 +319,7 @@ def generate_basic_results(options = {})
 
     ## add in virtual attributes that can be calculated from sql
     rbac_opts[:extra_cols] = va_sql_cols unless va_sql_cols.blank?
+    rbac_opts[:use_sql_view] = true if db_options && db_options[:use_sql_view]
 
     results, attrs = Rbac.search(rbac_opts)
     results = Metric::Helper.remove_duplicate_timestamps(results)

app/models/miq_report/search.rb

@@ -97,6 +97,7 @@ def paged_view_search(options = {})
                                   )
     search_options.merge!(:limit => limit, :offset => offset, :order => order) if order
     search_options[:extra_cols] = va_sql_cols if va_sql_cols.present?
+    search_options[:use_sql_view] = true if db_options && db_options[:use_sql_view]
 
     if options[:parent]
       targets = get_parent_targets(options[:parent], options[:association] || options[:parent_method])

lib/rbac/filterer.rb

@@ -254,10 +254,22 @@ def search(options = {})
 
       scope = include_references(scope, klass, include_for_find, exp_includes, skip_references)
       scope = scope.limit(limit).offset(offset) if attrs[:apply_limit_in_sql]
+
+      if inline_view?(options, scope)
+        inner_scope = scope.except(:select, :includes, :references)
+        scope.includes_values.each { |hash| inner_scope = add_joins(klass, inner_scope, hash) }
+        scope = scope.from(Arel.sql("(#{inner_scope.to_sql})").as(scope.table_name))
+                     .except(:offset, :limit, :order, :where)
+
+        # the auth_count needs to come from the inner query (the query with the limit)
+        if !options[:skip_counts] && (attrs[:apply_limit_in_sql] && limit)
+          auth_count = inner_scope.except(:offset, :limit, :order).count(:all)
+        end
+      end
       targets = scope
 
       unless options[:skip_counts]
-        auth_count = attrs[:apply_limit_in_sql] && limit ? targets.except(:offset, :limit, :order).count(:all) : targets.length
+        auth_count ||= attrs[:apply_limit_in_sql] && limit ? targets.except(:offset, :limit, :order).count(:all) : targets.length
       end
 
       if search_filter && targets && (!exp_attrs || !exp_attrs[:supported_by_sql])
@@ -286,6 +298,20 @@ def is_sti?(klass)
       klass.respond_to?(:finder_needs_type_condition?) ? klass.finder_needs_type_condition? : false
     end
 
+    # We would like to use an inline view if:
+    #   - we enabled viewing an inline view
+    #   - we have virtual attributes
+    #   - we are not bringing back the whole table (i.e. we do have a where() or a limit())
+    #   - we have a non qualified table name (otherwise our inline view will blow up)
+    #   - we are using a scope (instead of a relation - which doesn't do includes so well)
+    def inline_view?(options, scope)
+      options[:use_sql_view] &&
+        options[:extra_cols] &&
+        (scope.limit_value || scope.where_values_hash.present?) &&
+        !scope.table_name&.include?(".") &&
+        scope.respond_to?(:includes_values)
+    end
+
     # This is a very primitive way of determining whether we want to skip
     # adding references to the query.
     #
@@ -344,6 +370,24 @@ def include_references(scope, klass, include_for_find, exp_includes, skip)
       scope
     end
 
+    # @param includes [Array, Hash]
+    def add_joins(klass, scope, includes)
+      return scope unless includes
+      includes = Array(includes) unless includes.kind_of?(Enumerable)
+      includes.each do |association, value|
+        if table_include?(klass, association)
+          scope = value ? scope.left_outer_joins(association => value) : scope.left_outer_joins(association)
+        end
+      end
+      scope
+    end
+
+    # this is a reference to a non polymorphic table
+    def table_include?(target_klass, association)
+      reflection = target_klass.reflect_on_association(association)
+      reflection && !reflection.polymorphic?
+    end
+
     def polymorphic_include?(target_klass, includes)
       includes.keys.any? do |incld|
         reflection = target_klass.reflect_on_association(incld)

spec/lib/rbac/filterer_spec.rb

@@ -2090,6 +2090,77 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
         expect(results.length).to eq(VmdbDatabaseSetting.all.length)
       end
     end
+
+    describe "with inline view" do
+      let!(:tagged_group) { FactoryBot.create(:miq_group, :tenant => default_tenant) }
+      let!(:user)         { FactoryBot.create(:user, :miq_groups => [tagged_group]) }
+
+      before do
+        # the user can see production environments
+        tagged_group.entitlement = Entitlement.new
+        tagged_group.entitlement.set_belongsto_filters([])
+        tagged_group.entitlement.set_managed_filters([["/managed/environment/prod"]])
+        tagged_group.save!
+      end
+
+      it "handles added include from miq_expression" do
+        st = FactoryBot.create(:service_template)
+        service1, service2, _service3 = FactoryBot.create_list(:service, 3, :service_template => st)
+        service1.tag_with("/managed/environment/prod", :ns => "*")
+        service2.tag_with("/managed/environment/prod", :ns => "*")
+        service2.update_attributes(:service_template => nil)
+
+        # exclude service2 (no service template)
+        exp = YAML.safe_load("--- !ruby/object:MiqExpression
+        exp:
+          and:
+          - IS NOT EMPTY:
+              field: Service.service_template-name
+          - IS NOT EMPTY:
+              field: Service-name
+        ")
+        results = described_class.search(:class            => "Service",
+                                         :filter           => exp,
+                                         :limit            => 20,
+                                         :extra_cols       => "v_total_vms",
+                                         :use_sql_view     => true,
+                                         :user             => user,
+                                         :include_for_find => {:service_templates => :pictures},
+                                         :order            => "services.name desc")
+        expect(results.first).to eq([service1])
+        expect(results.last[:auth_count]).to eq(1)
+      end
+
+      it "handles ruby and sql miq_expression" do
+        root_service = FactoryBot.create(:service)
+        services1 = FactoryBot.create_list(:service, 4, :parent => root_service) # matches both (NOTE: more than limit)
+        services2 = FactoryBot.create_list(:service, 2) # matches rbac, and sql filter, not ruby filter
+        services1.each { |s| s.tag_with("/managed/environment/prod", :ns => "*") }
+        services2.each { |s| s.tag_with("/managed/environment/prod", :ns => "*") }
+        FactoryBot.create_list(:service, 1, :parent => root_service) # matches ruby and sql filter, not rbac
+
+        # expression with sql AND ruby
+        exp = YAML.safe_load("--- !ruby/object:MiqExpression
+        exp:
+          and:
+          - IS NOT EMPTY:
+              field: Service-name
+          - EQUAL:
+              field: Service-service_id
+              value: #{root_service.id}
+        ")
+
+        results = described_class.search(:class            => "Service",
+                                         :filter           => exp,
+                                         :limit            => 3,
+                                         :extra_cols       => "v_total_vms",
+                                         :use_sql_view     => true,
+                                         :user             => user,
+                                         :order            => "services.id")
+       expect(results.first).to eq(services1[0..2])
+       expect(results.last[:auth_count]).to eq(4)
+      end
+    end
   end
 
   describe ".filtered" do