From 9b1258d22d7c0b634b24d4803434049a8c981ffb Mon Sep 17 00:00:00 2001 From: Matias Grunberg Date: Tue, 30 Mar 2021 12:59:46 -0300 Subject: [PATCH 1/5] add support to optimizer_hints --- .../connection_adapters/sqlserver_adapter.rb | 4 ++ lib/arel/visitors/sqlserver.rb | 18 +++++++ test/cases/optimizer_hints_test_sqlserver.rb | 50 +++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 test/cases/optimizer_hints_test_sqlserver.rb diff --git a/lib/active_record/connection_adapters/sqlserver_adapter.rb b/lib/active_record/connection_adapters/sqlserver_adapter.rb index 867a66001..c30909aee 100644 --- a/lib/active_record/connection_adapters/sqlserver_adapter.rb +++ b/lib/active_record/connection_adapters/sqlserver_adapter.rb @@ -152,6 +152,10 @@ def supports_savepoints? true end + def supports_optimizer_hints? + true + end + def supports_lazy_transactions? true end diff --git a/lib/arel/visitors/sqlserver.rb b/lib/arel/visitors/sqlserver.rb index 680ee8be5..a62ce7fb4 100644 --- a/lib/arel/visitors/sqlserver.rb +++ b/lib/arel/visitors/sqlserver.rb @@ -80,6 +80,16 @@ def visit_Arel_Nodes_SelectStatement o, collector @select_statement = nil end + def visit_Arel_Nodes_SelectCore(o, collector) + collector = super + maybe_visit o.optimizer_hints, collector + end + + def visit_Arel_Nodes_OptimizerHints(o, collector) + hints = o.expr.map { |v| sanitize_as_option_clause(v) }.join(", ") + collector << "OPTION (#{hints})" + end + def visit_Arel_Table o, collector # Apparently, o.engine.connection can actually be a different adapter # than sqlserver. Can be removed if fixed in ActiveRecord. See: @@ -144,6 +154,10 @@ def collect_in_clause(left, right, collector) super end + def collect_optimizer_hints(o, collector) + collector + end + # SQLServer ToSql/Visitor (Additions) def visit_Arel_Nodes_SelectStatement_SQLServer_Lock collector, options = {} @@ -247,6 +261,10 @@ def remove_invalid_ordering_from_select_statement(node) node.orders = [] unless node.offset || node.limit end + + def sanitize_as_option_clause(value) + value.gsub(%r{OPTION \s* \( (.+) \)}x, "\\1") + end end end end diff --git a/test/cases/optimizer_hints_test_sqlserver.rb b/test/cases/optimizer_hints_test_sqlserver.rb new file mode 100644 index 000000000..0e1e1afab --- /dev/null +++ b/test/cases/optimizer_hints_test_sqlserver.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require "cases/helper_sqlserver" +require "models/company" + +class OptimizerHitsTestSQLServer < ActiveRecord::TestCase + fixtures :companies + + it "apply optimizations" do + assert_sql(%r{\ASELECT .+ FROM .+ OPTION \(HASH GROUP\)\z}) do + companies = Company.optimizer_hints("HASH GROUP") + companies = companies.distinct.select("firm_id") + assert_includes companies.explain, "| Hash Match | Aggregate |" + end + + assert_sql(%r{\ASELECT .+ FROM .+ OPTION \(ORDER GROUP\)\z}) do + companies = Company.optimizer_hints("ORDER GROUP") + companies = companies.distinct.select("firm_id") + assert_includes companies.explain, "| Stream Aggregate | Aggregate |" + end + end + + it "sanitize values" do + assert_sql(%r{\ASELECT .+ FROM .+ OPTION \(HASH GROUP\)\z}) do + companies = Company.optimizer_hints("OPTION (HASH GROUP)") + companies = companies.distinct.select("firm_id") + companies.to_a + end + + assert_sql(%r{\ASELECT .+ FROM .+ OPTION \(HASH GROUP\)\z}) do + companies = Company.optimizer_hints("OPTION(HASH GROUP)") + companies = companies.distinct.select("firm_id") + companies.to_a + end + + assert_sql(%r{\ASELECT .+ FROM .+ OPTION \(TABLE HINT \(\[companies\], INDEX\(1\)\)\)\z}) do + companies = Company.optimizer_hints("OPTION(TABLE HINT ([companies], INDEX(1)))") + companies = companies.distinct.select("firm_id") + companies.to_a + end + end + + it "skip optimization after unscope" do + assert_sql("SELECT DISTINCT [companies].[firm_id] FROM [companies]") do + companies = Company.optimizer_hints("HASH GROUP") + companies = companies.distinct.select("firm_id") + companies.unscope(:optimizer_hints).load + end + end +end From 70c0891ee40581352d42fb8e16a259eb77f21ec7 Mon Sep 17 00:00:00 2001 From: Matias Grunberg Date: Tue, 30 Mar 2021 20:28:29 -0300 Subject: [PATCH 2/5] Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d3100aca..bd6539a2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,8 @@ #### Added - [#855](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/855) Add helpers to create/change/drop a schema. -- [#857](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/857) Included WAITFOR as read query type. +- [#857](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/857) Included WAITFOR as read query type. +- [#865](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/865) Implemented optimizer hints. ## v6.0.1 From 54dd652f715aef36a762c9d93dced87b91e7a82d Mon Sep 17 00:00:00 2001 From: Matias Grunberg Date: Tue, 30 Mar 2021 20:31:19 -0300 Subject: [PATCH 3/5] add more specs --- test/cases/optimizer_hints_test_sqlserver.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/cases/optimizer_hints_test_sqlserver.rb b/test/cases/optimizer_hints_test_sqlserver.rb index 0e1e1afab..c113e9890 100644 --- a/test/cases/optimizer_hints_test_sqlserver.rb +++ b/test/cases/optimizer_hints_test_sqlserver.rb @@ -20,6 +20,22 @@ class OptimizerHitsTestSQLServer < ActiveRecord::TestCase end end + it "apply multiple optimizations" do + assert_sql(%r{\ASELECT .+ FROM .+ OPTION \(HASH GROUP, FAST 1\)\z}) do + companies = Company.optimizer_hints("HASH GROUP", "FAST 1") + companies = companies.distinct.select("firm_id") + assert_includes companies.explain, "| Hash Match | Flow Distinct |" + end + end + + it "support subqueries" do + assert_sql(%r{.*'SELECT COUNT\(count_column\) FROM \(SELECT .*\) subquery_for_count OPTION \(MAXDOP 2\)'.*}) do + companies = Company.optimizer_hints("MAXDOP 2") + companies = companies.select(:id).where(firm_id: [0, 1]).limit(3) + assert_equal 3, companies.count + end + end + it "sanitize values" do assert_sql(%r{\ASELECT .+ FROM .+ OPTION \(HASH GROUP\)\z}) do companies = Company.optimizer_hints("OPTION (HASH GROUP)") From 45165681e6a401ceaf791ea9d4222653e15d11d1 Mon Sep 17 00:00:00 2001 From: Matias Grunberg Date: Tue, 30 Mar 2021 21:58:53 -0300 Subject: [PATCH 4/5] coerce tests --- test/cases/coerced_tests.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/cases/coerced_tests.rb b/test/cases/coerced_tests.rb index 3e8181a3a..bc221e98d 100644 --- a/test/cases/coerced_tests.rb +++ b/test/cases/coerced_tests.rb @@ -982,6 +982,21 @@ def test_reverse_arel_assoc_order_with_function_coerced end end +module ActiveRecord + class RelationTest < ActiveRecord::TestCase + # Skipping this test. SQL Server doesn't support optimizer hint as comments + coerce_tests! :test_relation_with_optimizer_hints_filters_sql_comment_delimiters + + coerce_tests! :test_does_not_duplicate_optimizer_hints_on_merge + def test_does_not_duplicate_optimizer_hints_on_merge_coerced + escaped_table = Post.connection.quote_table_name("posts") + expected = "SELECT #{escaped_table}.* FROM #{escaped_table} OPTION (OMGHINT)" + query = Post.optimizer_hints("OMGHINT").merge(Post.optimizer_hints("OMGHINT")).to_sql + assert_equal expected, query + end + end +end + require "models/post" class SanitizeTest < ActiveRecord::TestCase # Use nvarchar string (N'') in assert From 16bc64e1fed330b4c3726f27491c2618f5160058 Mon Sep 17 00:00:00 2001 From: Matias Grunberg Date: Wed, 31 Mar 2021 10:01:55 -0300 Subject: [PATCH 5/5] case insensitive sanitize --- lib/arel/visitors/sqlserver.rb | 2 +- test/cases/optimizer_hints_test_sqlserver.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/arel/visitors/sqlserver.rb b/lib/arel/visitors/sqlserver.rb index a62ce7fb4..2642c6950 100644 --- a/lib/arel/visitors/sqlserver.rb +++ b/lib/arel/visitors/sqlserver.rb @@ -263,7 +263,7 @@ def remove_invalid_ordering_from_select_statement(node) end def sanitize_as_option_clause(value) - value.gsub(%r{OPTION \s* \( (.+) \)}x, "\\1") + value.gsub(%r{OPTION \s* \( (.+) \)}xi, "\\1") end end end diff --git a/test/cases/optimizer_hints_test_sqlserver.rb b/test/cases/optimizer_hints_test_sqlserver.rb index c113e9890..ba0438c46 100644 --- a/test/cases/optimizer_hints_test_sqlserver.rb +++ b/test/cases/optimizer_hints_test_sqlserver.rb @@ -54,6 +54,12 @@ class OptimizerHitsTestSQLServer < ActiveRecord::TestCase companies = companies.distinct.select("firm_id") companies.to_a end + + assert_sql(%r{\ASELECT .+ FROM .+ OPTION \(HASH GROUP\)\z}) do + companies = Company.optimizer_hints("Option(HASH GROUP)") + companies = companies.distinct.select("firm_id") + companies.to_a + end end it "skip optimization after unscope" do