Support Optimizer Hints

[kamipo]

We as Arm Treasure Data are using Optimizer Hints with a monkey patch
(https://gist.github.com/kamipo/4c8539f0ce4acf85075cf5a6b0d9712e),
especially in order to use MAX_EXECUTION_TIME (refer #31129).

Example:

class Job < ApplicationRecord
  default_scope { optimizer_hints("MAX_EXECUTION_TIME(50000) NO_INDEX_MERGE(jobs)") }
end

Optimizer Hints is supported not only for MySQL but also for most
databases (PostgreSQL on RDS, Oracle, SQL Server, etc), it is really
helpful to turn heavy queries for large scale applications.

[kaspth]

Should the exposed API be closer to something like this?

class Job < ApplicationRecord
  query_optimize max_execution_time: 50.minutes, no_index_merge: true
end

Then we’d add an option for other hints as well.

[jeremy]

Very nice, and welcome.

[jeremy]

@kaspth In our usage, we provide optimizer hints for specific queries and scopes. A class-level wrapper declaration could work for default scoping, but the lower level relation support is desirable.

[rafaelfranca]

👏

[mattyoho]

Hi, @kamipo! 👋

I don't want to derail this pull request, so my apologies if this feels derailing.

I thought it was amusing that this pull request and #35617, which aimed to add plain SQL comment support to ActiveRecord::Relation, were opened within an hour or two of one another given their similarity.

@jeremy's comment at #35615 (comment) caused me to think about what it would take to generalize #35617 a bit so that it could act as underpinning for the new #optimizer_hints query method.

Given how similar the concepts are, it seems as though there would be value in sharing an abstraction.

I've since made some changes over in #35617, including generalizing an Arel::Nodes::Annotation node and adding hooks to customize how a visitor might generate output from that node.

Here's a small synthetic patch demonstrating how I think it could work:

diff --git a/activerecord/lib/arel/select_manager.rb b/activerecord/lib/arel/select_manager.rb
index 8a8bd3b..91db035 100644
--- a/activerecord/lib/arel/select_manager.rb
+++ b/activerecord/lib/arel/select_manager.rb
@@ -148,7 +148,7 @@ module Arel # :nodoc: all

     def optimizer_hints(*hints)
       unless hints.empty?
-        @ctx.optimizer_hints = Arel::Nodes::OptimizerHints.new(hints)
+        @ctx.optimizer_hints = Arel::Nodes::Annotation.new(hints, hint: true)
       end
       self
     end
@@ -263,4 +263,4 @@ module Arel # :nodoc: all
         end
       end
   end
 end
diff --git a/activerecord/lib/arel/visitors/ibm_db.rb b/activerecord/lib/arel/visitors/ibm_db.rb
index 73166054da..05908b0fc0 100644
--- a/activerecord/lib/arel/visitors/ibm_db.rb
+++ b/activerecord/lib/arel/visitors/ibm_db.rb
@@ -16,6 +16,15 @@ def is_distinct_from(o, collector)
           collector = visit [o.left, o.right, 0, 1], collector
           collector << ")"
         end
+
+        HINT_START       = "/* <OPTGUIDELINES>"  # :nodoc:
+        HINT_END         = "</OPTGUIDELINES> */" # :nodoc:
+        HINT_SEPARATOR   = ""                    # :nodoc:
+
+        def visit_Arel_Nodes_SelectCore(o, collector)
+          collector = super
+          maybe_visit o.optimizer_hints, collector
+        end
     end
   end
 end
diff --git a/activerecord/test/cases/arel/visitors/ibm_db_test.rb b/activerecord/test/cases/arel/visitors/ibm_db_test.rb
index 2ddbec3266..b75e51908a 100644
--- a/activerecord/test/cases/arel/visitors/ibm_db_test.rb
+++ b/activerecord/test/cases/arel/visitors/ibm_db_test.rb
@@ -68,6 +68,13 @@ def compile(node)
           sql.must_be_like %{ "users"."name" IS NOT NULL }
         end
       end
+
+      describe "Nodes::Annotation" do
+        it "should wrap a hint in multiline OPTGUIDELINES delimiters" do
+          node = Arel::Nodes::Annotation.new "omg", hint: true
+          compile(node).must_be_like %{ /\*\ <OPTGUIDELINES\>omg\</OPTGUIDELINES\> \*/ }
+        end
+      end
     end
   end
 end

There would still be a user-facing ActiveRecord::Relation#optimizer_hints API addition for adding hints to relations, and Arel::Nodes::SelectCore would still hold a distinct optimizer_hints property so that a visitor could choose where in the generated output to place the given hints.

I'm curious what you would think of an approach like this. Thank you!

/cc @jeremy @rafaelfranca

[kamipo]

Thanks for your comment @mattyoho.

I agree that perhaps optimizer hints and comment (annotation) could share something code.

But for the SQL visitor, I'd prefer simplest separated visitor methods

        def visit_Arel_Nodes_Annotation(o, collector)
          collector << "/* #{o.expr.join(" ")} */"
        end

        def visit_Arel_Nodes_OptimizerHints(o, collector)
          collector << "/*+ #{o.expr.join(" ")} */"
        end

than one complex visitor method

        ANNOTATION_START = "/* "  # :nodoc:
        ANNOTATION_END   = " */"  # :nodoc:
        HINT_START       = "/*+ " # :nodoc:
        HINT_END         = " */"  # :nodoc:
        HINT_SEPARATOR   = SPACE  # :nodoc:

        def visit_Arel_Nodes_Annotation(o, collector)
          collector << (o.hint? ? self.class::HINT_START : self.class::ANNOTATION_START)
          o.values.each do |value|
            visit(value, collector)
            collector << self.class::HINT_SEPARATOR if o.hint?
          end
          collector << (o.hint? ? self.class::HINT_END : self.class::ANNOTATION_END)
        end

.

[mattyoho]

@kamipo wrote:

But for the SQL visitor, I'd prefer simplest separated visitor methods [...] than one complex visitor method

That's an excellent point. It's also probably much more in line with the visitor design to use inheritance with simple methods, as well.

Thank you for considering my comment.