From a7b1adb2629228c39a6d2fcdb8ac455e6039b5b7 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 6 Nov 2025 20:36:05 +0000 Subject: [PATCH 1/3] Fixed query logging so that filter parameters are respected --- .../sqlserver/database_statements.rb | 104 ++++---- test/cases/coerced_tests.rb | 232 ++++-------------- test/cases/optimizer_hints_test_sqlserver.rb | 2 +- test/cases/showplan_test_sqlserver.rb | 4 +- test/cases/specific_schema_test_sqlserver.rb | 12 +- test/support/query_assertions.rb | 22 ++ 6 files changed, 122 insertions(+), 254 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index 096bff4ff..e7262774e 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -14,15 +14,20 @@ def write_query?(sql) # :nodoc: end def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:, batch:) + unless binds.nil? || binds.empty? + types, params = sp_executesql_types_and_parameters(binds) + sql = sp_executesql_sql(sql, types, params, notification_payload[:name]) + end + id_insert_table_name = query_requires_identity_insert?(sql) result, affected_rows = if id_insert_table_name - with_identity_insert_enabled(id_insert_table_name, raw_connection) do - internal_exec_sql_query(sql, raw_connection) - end - else - internal_exec_sql_query(sql, raw_connection) - end + with_identity_insert_enabled(id_insert_table_name, raw_connection) do + internal_exec_sql_query(sql, raw_connection) + end + else + internal_exec_sql_query(sql, raw_connection) + end verified! notification_payload[:affected_rows] = affected_rows @@ -48,15 +53,6 @@ def affected_rows_from_results_or_handle(raw_result, handle) affected_rows(raw_result) || handle.affected_rows end - def raw_execute(sql, name = nil, binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true, batch: false) - unless binds.nil? || binds.empty? - types, params = sp_executesql_types_and_parameters(binds) - sql = sp_executesql_sql(sql, types, params, name) - end - - super - end - def internal_exec_sql_query(sql, conn) handle = internal_raw_execute(sql, conn) results = handle_to_names_and_values(handle, ar_result: true) @@ -235,10 +231,10 @@ def merge_insert_values_list(insert:, insert_all:) def execute_procedure(proc_name, *variables) vars = if variables.any? && variables.first.is_a?(Hash) - variables.first.map { |k, v| "@#{k} = #{quote(v)}" } - else - variables.map { |v| quote(v) } - end.join(", ") + variables.first.map { |k, v| "@#{k} = #{quote(v)}" } + else + variables.map { |v| quote(v) } + end.join(", ") sql = "EXEC #{proc_name} #{vars}".strip log(sql, "Execute Procedure") do |notification_payload| @@ -344,35 +340,35 @@ def sql_for_insert(sql, pk, binds, returning) end sql = if pk && use_output_inserted? && !database_prefix_remote_server? - table_name ||= get_table_name(sql) - exclude_output_inserted = exclude_output_inserted_table_name?(table_name, sql) - - if exclude_output_inserted - pk_and_types = Array(pk).map do |subkey| - { - quoted: SQLServer::Utils.extract_identifiers(subkey).quoted, - id_sql_type: exclude_output_inserted_id_sql_type(subkey, exclude_output_inserted) - } - end - - <<~SQL.squish + table_name ||= get_table_name(sql) + exclude_output_inserted = exclude_output_inserted_table_name?(table_name, sql) + + if exclude_output_inserted + pk_and_types = Array(pk).map do |subkey| + { + quoted: SQLServer::Utils.extract_identifiers(subkey).quoted, + id_sql_type: exclude_output_inserted_id_sql_type(subkey, exclude_output_inserted) + } + end + + <<~SQL.squish DECLARE @ssaIdInsertTable table (#{pk_and_types.map { |pk_and_type| "#{pk_and_type[:quoted]} #{pk_and_type[:id_sql_type]}" }.join(", ")}); #{sql.dup.insert sql.index(/ (DEFAULT )?VALUES/i), " OUTPUT #{pk_and_types.map { |pk_and_type| "INSERTED.#{pk_and_type[:quoted]}" }.join(", ")} INTO @ssaIdInsertTable"} SELECT #{pk_and_types.map { |pk_and_type| "CAST(#{pk_and_type[:quoted]} AS #{pk_and_type[:id_sql_type]}) #{pk_and_type[:quoted]}" }.join(", ")} FROM @ssaIdInsertTable SQL - else - returning_columns = returning || Array(pk) - - if returning_columns.any? - returning_columns_statements = returning_columns.map { |c| " INSERTED.#{SQLServer::Utils.extract_identifiers(c).quoted}" } - sql.dup.insert sql.index(/ (DEFAULT )?VALUES/i), " OUTPUT" + returning_columns_statements.join(",") - else - sql - end - end - else - "#{sql}; SELECT CAST(SCOPE_IDENTITY() AS bigint) AS Ident" - end + else + returning_columns = returning || Array(pk) + + if returning_columns.any? + returning_columns_statements = returning_columns.map { |c| " INSERTED.#{SQLServer::Utils.extract_identifiers(c).quoted}" } + sql.dup.insert sql.index(/ (DEFAULT )?VALUES/i), " OUTPUT" + returning_columns_statements.join(",") + else + sql + end + end + else + "#{sql}; SELECT CAST(SCOPE_IDENTITY() AS bigint) AS Ident" + end [sql, binds] end @@ -541,16 +537,16 @@ def build_sql_for_returning(insert:, insert_all:) return "" unless insert_all.returning returning_values_sql = if insert_all.returning.is_a?(String) - insert_all.returning - else - Array(insert_all.returning).map do |attribute| - if insert.model.attribute_alias?(attribute) - "INSERTED.#{quote_column_name(insert.model.attribute_alias(attribute))} AS #{quote_column_name(attribute)}" - else - "INSERTED.#{quote_column_name(attribute)}" - end - end.join(",") - end + insert_all.returning + else + Array(insert_all.returning).map do |attribute| + if insert.model.attribute_alias?(attribute) + "INSERTED.#{quote_column_name(insert.model.attribute_alias(attribute))} AS #{quote_column_name(attribute)}" + else + "INSERTED.#{quote_column_name(attribute)}" + end + end.join(",") + end " OUTPUT #{returning_values_sql}" end diff --git a/test/cases/coerced_tests.rb b/test/cases/coerced_tests.rb index a7429f7d1..6ac8d71a9 100644 --- a/test/cases/coerced_tests.rb +++ b/test/cases/coerced_tests.rb @@ -261,7 +261,7 @@ def test_belongs_to_with_primary_key_joins_on_correct_column_coerced def test_belongs_to_coerced client = Client.find(3) first_firm = companies(:first_firm) - assert_queries_match(/FETCH NEXT @(\d) ROWS ONLY(.)*@\1 = 1/) do + assert_queries_and_values_match(/FETCH NEXT @3 ROWS ONLY/, ['Firm', 'Agency', 1, 1]) do assert_equal first_firm, client.firm assert_equal first_firm.name, client.firm.name end @@ -270,21 +270,6 @@ def test_belongs_to_coerced module ActiveRecord class BindParameterTest < ActiveRecord::TestCase - # Same as original coerced test except log is found using `EXEC sp_executesql` wrapper. - coerce_tests! :test_binds_are_logged - def test_binds_are_logged_coerced - sub = Arel::Nodes::BindParam.new(1) - binds = [Relation::QueryAttribute.new("id", 1, Type::Value.new)] - sql = "select * from topics where id = #{sub.to_sql}" - - @connection.exec_query(sql, "SQL", binds) - - logged_sql = "EXEC sp_executesql N'#{sql}', N'#{sub.to_sql} int', #{sub.to_sql} = 1" - message = @subscriber.calls.find { |args| args[4][:sql] == logged_sql } - - assert_equal binds, message[4][:binds] - end - # SQL Server adapter does not use a statement cache as query plans are already reused using `EXEC sp_executesql`. coerce_tests! :test_statement_cache coerce_tests! :test_statement_cache_with_query_cache @@ -292,55 +277,6 @@ def test_binds_are_logged_coerced coerce_tests! :test_statement_cache_with_find_by coerce_tests! :test_statement_cache_with_in_clause coerce_tests! :test_statement_cache_with_sql_string_literal - - # Same as original coerced test except prepared statements include `EXEC sp_executesql` wrapper. - coerce_tests! :test_bind_params_to_sql_with_prepared_statements, :test_bind_params_to_sql_with_unprepared_statements - def test_bind_params_to_sql_with_prepared_statements_coerced - assert_bind_params_to_sql_coerced(prepared: true) - end - - def test_bind_params_to_sql_with_unprepared_statements_coerced - @connection.unprepared_statement do - assert_bind_params_to_sql_coerced(prepared: false) - end - end - - private - - def assert_bind_params_to_sql_coerced(prepared:) - table = Author.quoted_table_name - pk = "#{table}.#{Author.quoted_primary_key}" - - # prepared_statements: true - # - # EXEC sp_executesql N'SELECT [authors].* FROM [authors] WHERE [authors].[id] IN (@0, @1, @2) OR [authors].[id] IS NULL)', N'@0 bigint, @1 bigint, @2 bigint', @0 = 1, @1 = 2, @2 = 3 - # - # prepared_statements: false - # - # SELECT [authors].* FROM [authors] WHERE ([authors].[id] IN (1, 2, 3) OR [authors].[id] IS NULL) - # - sql_unprepared = "SELECT #{table}.* FROM #{table} WHERE (#{pk} IN (#{bind_params(1..3)}) OR #{pk} IS NULL)" - sql_prepared = "EXEC sp_executesql N'SELECT #{table}.* FROM #{table} WHERE (#{pk} IN (#{bind_params(1..3)}) OR #{pk} IS NULL)', N'@0 bigint, @1 bigint, @2 bigint', @0 = 1, @1 = 2, @2 = 3" - - authors = Author.where(id: [1, 2, 3, nil]) - assert_equal sql_unprepared, @connection.to_sql(authors.arel) - assert_queries_match(prepared ? sql_prepared : sql_unprepared) { assert_equal 3, authors.length } - - # prepared_statements: true - # - # EXEC sp_executesql N'SELECT [authors].* FROM [authors] WHERE [authors].[id] IN (@0, @1, @2)', N'@0 bigint, @1 bigint, @2 bigint', @0 = 1, @1 = 2, @2 = 3 - # - # prepared_statements: false - # - # SELECT [authors].* FROM [authors] WHERE [authors].[id] IN (1, 2, 3) - # - sql_unprepared = "SELECT #{table}.* FROM #{table} WHERE #{pk} IN (#{bind_params(1..3)})" - sql_prepared = "EXEC sp_executesql N'SELECT #{table}.* FROM #{table} WHERE #{pk} IN (#{bind_params(1..3)})', N'@0 bigint, @1 bigint, @2 bigint', @0 = 1, @1 = 2, @2 = 3" - - authors = Author.where(id: [1, 2, 3, 9223372036854775808]) - assert_equal sql_unprepared, @connection.to_sql(authors.arel) - assert_queries_match(prepared ? sql_prepared : sql_unprepared) { assert_equal 3, authors.length } - end end end @@ -461,11 +397,11 @@ def test_select_avg_with_group_by_as_virtual_attribute_with_ar_coerced rails_core = companies(:rails_core) account = Account - .select(:firm_id, "AVG(CAST(credit_limit AS DECIMAL)) AS avg_credit_limit") - .where(firm: rails_core) - .group(:firm_id) - .order(:firm_id) - .take! + .select(:firm_id, "AVG(CAST(credit_limit AS DECIMAL)) AS avg_credit_limit") + .where(firm: rails_core) + .group(:firm_id) + .order(:firm_id) + .take! # id was not selected, so it should be nil # (cannot select id because it wasn't used in the GROUP BY clause) @@ -512,11 +448,11 @@ def test_select_avg_with_joins_and_group_by_as_virtual_attribute_with_ar_coerced rails_core = companies(:rails_core) firm = DependentFirm - .select("companies.*", "AVG(CAST(accounts.credit_limit AS DECIMAL)) AS avg_credit_limit") - .where(id: rails_core) - .joins(:account) - .group(:id, :type, :firm_id, :firm_name, :name, :client_of, :rating, :account_id, :description, :status) - .take! + .select("companies.*", "AVG(CAST(accounts.credit_limit AS DECIMAL)) AS avg_credit_limit") + .where(id: rails_core) + .joins(:account) + .group(:id, :type, :firm_id, :firm_name, :name, :client_of, :rating, :account_id, :description, :status) + .take! # all the DependentFirm attributes should be present assert_equal rails_core, firm @@ -531,7 +467,7 @@ def test_select_avg_with_joins_and_group_by_as_virtual_attribute_with_ar_coerced def test_limit_is_kept_coerced queries = capture_sql { Account.limit(1).count } assert_equal 1, queries.length - assert_match(/ORDER BY \[accounts\]\.\[id\] ASC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY.*@0 = 1/, queries.first) + assert_match(/ORDER BY \[accounts\]\.\[id\] ASC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY/, queries.first) end # Match SQL Server limit implementation @@ -539,7 +475,7 @@ def test_limit_is_kept_coerced def test_limit_with_offset_is_kept_coerced queries = capture_sql { Account.limit(1).offset(1).count } assert_equal 1, queries.length - assert_match(/ORDER BY \[accounts\]\.\[id\] ASC OFFSET @0 ROWS FETCH NEXT @1 ROWS ONLY.*@0 = 1, @1 = 1/, queries.first) + assert_match(/ORDER BY \[accounts\]\.\[id\] ASC OFFSET @0 ROWS FETCH NEXT @1 ROWS ONLY/, queries.first) end # SQL Server needs an alias for the calculated column @@ -1061,14 +997,14 @@ class FinderTest < ActiveRecord::TestCase # We have implicit ordering, via FETCH. coerce_tests! %r{doesn't have implicit ordering}, - :test_find_doesnt_have_implicit_ordering + :test_find_doesnt_have_implicit_ordering # Assert SQL Server limit implementation coerce_tests! :test_take_and_first_and_last_with_integer_should_use_sql_limit def test_take_and_first_and_last_with_integer_should_use_sql_limit_coerced - assert_queries_match(/OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY.* @0 = 3/) { Topic.take(3).entries } - assert_queries_match(/OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY.* @0 = 2/) { Topic.first(2).entries } - assert_queries_match(/OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY.* @0 = 5/) { Topic.last(5).entries } + assert_queries_and_values_match(/OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY/, [3]) { Topic.take(3).entries } + assert_queries_and_values_match(/OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY/, [2]) { Topic.first(2).entries } + assert_queries_and_values_match(/OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY/, [5]) { Topic.last(5).entries } end # This fails only when run in the full test suite task. Just taking it out of the mix. @@ -1099,7 +1035,7 @@ def test_condition_local_time_interpolation_with_default_timezone_utc_coerced # Check for `FETCH NEXT x ROWS` rather then `LIMIT`. coerce_tests! :test_include_on_unloaded_relation_with_match def test_include_on_unloaded_relation_with_match_coerced - assert_queries_match(/1 AS one.*FETCH NEXT @2 ROWS ONLY.*@2 = 1/) do + assert_queries_match(/1 AS one.*FETCH NEXT @2 ROWS ONLY/) do assert_equal true, Customer.where(name: "David").include?(customers(:david)) end end @@ -1107,7 +1043,7 @@ def test_include_on_unloaded_relation_with_match_coerced # Check for `FETCH NEXT x ROWS` rather then `LIMIT`. coerce_tests! :test_include_on_unloaded_relation_without_match def test_include_on_unloaded_relation_without_match_coerced - assert_queries_match(/1 AS one.*FETCH NEXT @2 ROWS ONLY.*@2 = 1/) do + assert_queries_match(/1 AS one.*FETCH NEXT @2 ROWS ONLY/) do assert_equal false, Customer.where(name: "David").include?(customers(:mary)) end end @@ -1115,7 +1051,7 @@ def test_include_on_unloaded_relation_without_match_coerced # Check for `FETCH NEXT x ROWS` rather then `LIMIT`. coerce_tests! :test_member_on_unloaded_relation_with_match def test_member_on_unloaded_relation_with_match_coerced - assert_queries_match(/1 AS one.*FETCH NEXT @2 ROWS ONLY.*@2 = 1/) do + assert_queries_match(/1 AS one.*FETCH NEXT @2 ROWS ONLY/) do assert_equal true, Customer.where(name: "David").member?(customers(:david)) end end @@ -1123,7 +1059,7 @@ def test_member_on_unloaded_relation_with_match_coerced # Check for `FETCH NEXT x ROWS` rather then `LIMIT`. coerce_tests! :test_member_on_unloaded_relation_without_match def test_member_on_unloaded_relation_without_match_coerced - assert_queries_match(/1 AS one.*FETCH NEXT @2 ROWS ONLY.*@2 = 1/) do + assert_queries_match(/1 AS one.*FETCH NEXT @2 ROWS ONLY/) do assert_equal false, Customer.where(name: "David").member?(customers(:mary)) end end @@ -1138,7 +1074,7 @@ def test_implicit_order_column_is_configurable_with_a_single_value_coerced assert_equal topics(:third), Topic.last c = Topic.lease_connection - assert_queries_match(/ORDER BY #{Regexp.escape(c.quote_table_name("topics.title"))} DESC, #{Regexp.escape(c.quote_table_name("topics.id"))} DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY.*@0 = 1/i) { + assert_queries_and_values_match(/ORDER BY #{Regexp.escape(c.quote_table_name("topics.title"))} DESC, #{Regexp.escape(c.quote_table_name("topics.id"))} DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY/i, [1]) { Topic.last } ensure @@ -1178,7 +1114,7 @@ def test_implicit_order_set_to_primary_key_coerced Topic.implicit_order_column = "id" c = Topic.lease_connection - assert_queries_match(/ORDER BY #{Regexp.escape(c.quote_table_name("topics.id"))} DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY.*@0 = 1/i) { + assert_queries_and_values_match(/ORDER BY #{Regexp.escape(c.quote_table_name("topics.id"))} DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY/i, [1]) { Topic.last } ensure @@ -1193,7 +1129,7 @@ def test_implicit_order_for_model_without_primary_key_coerced c = NonPrimaryKey.lease_connection - assert_queries_match(/ORDER BY #{Regexp.escape(c.quote_table_name("non_primary_keys.created_at"))} DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY.*@0 = 1/i) { + assert_queries_and_values_match(/ORDER BY #{Regexp.escape(c.quote_table_name("non_primary_keys.created_at"))} DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY/i, [1]) { NonPrimaryKey.last } ensure @@ -1203,7 +1139,7 @@ def test_implicit_order_for_model_without_primary_key_coerced # Check for `FETCH NEXT x ROWS` rather then `LIMIT`. coerce_tests! :test_member_on_unloaded_relation_with_composite_primary_key def test_member_on_unloaded_relation_with_composite_primary_key_coerced - assert_queries_match(/1 AS one.* FETCH NEXT @(\d) ROWS ONLY.*@\1 = 1/) do + assert_queries_match(/1 AS one.* FETCH NEXT @3 ROWS ONLY/) do book = cpk_books(:cpk_great_author_first_book) assert Cpk::Book.where(title: "The first book").member?(book) end @@ -1218,8 +1154,7 @@ def test_implicit_order_column_prepends_query_constraints_coerced quoted_color = Regexp.escape(c.quote_table_name("clothing_items.color")) quoted_descrption = Regexp.escape(c.quote_table_name("clothing_items.description")) - assert_queries_match(/ORDER BY #{quoted_descrption} ASC, #{quoted_type} ASC, #{quoted_color} ASC OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY.*@\1 = 1/i) do - assert_kind_of ClothingItem, ClothingItem.first + assert_queries_match(/ORDER BY #{quoted_descrption} ASC, #{quoted_type} ASC, #{quoted_color} ASC OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY/i) do assert_kind_of ClothingItem, ClothingItem.first end ensure ClothingItem.implicit_order_column = nil @@ -1232,7 +1167,7 @@ def test_implicit_order_column_prepends_query_constraints_coerced quoted_type = Regexp.escape(c.quote_table_name("clothing_items.clothing_type")) quoted_color = Regexp.escape(c.quote_table_name("clothing_items.color")) - assert_queries_match(/ORDER BY #{quoted_type} DESC, #{quoted_color} DESC OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY.*@\1 = 1/i) do + assert_queries_match(/ORDER BY #{quoted_type} DESC, #{quoted_color} DESC OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY/i) do assert_kind_of ClothingItem, ClothingItem.last end end @@ -1244,7 +1179,7 @@ def test_implicit_order_column_prepends_query_constraints_coerced quoted_type = Regexp.escape(c.quote_table_name("clothing_items.clothing_type")) quoted_color = Regexp.escape(c.quote_table_name("clothing_items.color")) - assert_queries_match(/ORDER BY #{quoted_type} ASC, #{quoted_color} ASC OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY.*@\1 = 1/i) do + assert_queries_match(/ORDER BY #{quoted_type} ASC, #{quoted_color} ASC OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY/i) do assert_kind_of ClothingItem, ClothingItem.first end end @@ -1257,7 +1192,7 @@ def test_implicit_order_column_reorders_query_constraints_coerced quoted_type = Regexp.escape(c.quote_table_name("clothing_items.clothing_type")) quoted_color = Regexp.escape(c.quote_table_name("clothing_items.color")) - assert_queries_match(/ORDER BY #{quoted_color} ASC, #{quoted_type} ASC OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY.*@\1 = 1/i) do + assert_queries_match(/ORDER BY #{quoted_color} ASC, #{quoted_type} ASC OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY/i) do assert_kind_of ClothingItem, ClothingItem.first end ensure @@ -1267,7 +1202,7 @@ def test_implicit_order_column_reorders_query_constraints_coerced # Check for `FETCH NEXT x ROWS` rather then `LIMIT`. coerce_tests! :test_include_on_unloaded_relation_with_composite_primary_key def test_include_on_unloaded_relation_with_composite_primary_key_coerced - assert_queries_match(/1 AS one.*OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY.*@\1 = 1/) do + assert_queries_match(/1 AS one.*OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY/) do book = cpk_books(:cpk_great_author_first_book) assert Cpk::Book.where(title: "The first book").include?(book) end @@ -1277,11 +1212,11 @@ def test_include_on_unloaded_relation_with_composite_primary_key_coerced coerce_tests! :test_nth_to_last_with_order_uses_limit def test_nth_to_last_with_order_uses_limit_coerced c = Topic.lease_connection - assert_queries_match(/ORDER BY #{Regexp.escape(c.quote_table_name("topics.id"))} DESC OFFSET @(\d) ROWS FETCH NEXT @(\d) ROWS ONLY.*@\1 = 1.*@\2 = 1/i) do + assert_queries_match(/ORDER BY #{Regexp.escape(c.quote_table_name("topics.id"))} DESC OFFSET @(\d) ROWS FETCH NEXT @(\d) ROWS ONLY/i) do Topic.second_to_last end - assert_queries_match(/ORDER BY #{Regexp.escape(c.quote_table_name("topics.updated_at"))} DESC OFFSET @(\d) ROWS FETCH NEXT @(\d) ROWS ONLY.*@\1 = 1.*@\2 = 1/i) do + assert_queries_match(/ORDER BY #{Regexp.escape(c.quote_table_name("topics.updated_at"))} DESC OFFSET @(\d) ROWS FETCH NEXT @(\d) ROWS ONLY/i) do Topic.order(:updated_at).second_to_last end end @@ -1332,7 +1267,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_has_one_coerced firm = companies(:first_firm) first_account = Account.find(1) - assert_queries_match(/FETCH NEXT @(\d) ROWS ONLY(.)*@\1 = 1/) do + assert_queries_match(/FETCH NEXT @(\d) ROWS ONLY/) do assert_equal first_account, firm.account assert_equal first_account.credit_limit, firm.account.credit_limit end @@ -1344,7 +1279,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase coerce_tests! :test_has_one_through_executes_limited_query def test_has_one_through_executes_limited_query_coerced boring_club = clubs(:boring_club) - assert_queries_match(/FETCH NEXT @(\d) ROWS ONLY(.)*@\1 = 1/) do + assert_queries_match(/FETCH NEXT @(\d) ROWS ONLY/) do assert_equal boring_club, @member.general_club end end @@ -1546,7 +1481,7 @@ def test_reorder_with_first_coerced def test_multiple_where_and_having_clauses_coerced post = Post.first having_then_where = Post.having(id: post.id).where(title: post.title) - .having(id: post.id).where(title: post.title).group(:id).select(:id) + .having(id: post.id).where(title: post.title).group(:id).select(:id) assert_equal [post], having_then_where end @@ -1565,7 +1500,7 @@ def test_having_with_binds_for_both_where_and_having # Find any limit via our expression. coerce_tests! %r{relations don't load all records in #inspect} def test_relations_dont_load_all_records_in_inspect_coerced - assert_queries_match(/NEXT @0 ROWS.*@0 = \d+/) do + assert_queries_match(/NEXT @0 ROWS/) do Post.all.inspect end end @@ -1678,7 +1613,7 @@ def test_dump_schema_versions_outputs_lexically_reverse_ordered_versions_regardl schema_info = ActiveRecord::Base.lease_connection.dump_schema_versions expected = <<~STR - INSERT INTO #{quote_table_name("schema_migrations")} (version) VALUES + INSERT INTO #{ActiveRecord::Base.lease_connection.quote_table_name("schema_migrations")} (version) VALUES (N'20100301010101'), (N'20100201010101'), (N'20100101010101'); @@ -2257,7 +2192,7 @@ def test_merge_doesnt_duplicate_same_clauses_coerced non_mary_and_bob = Author.where.not(id: [mary, bob]) author_id = Author.lease_connection.quote_table_name("authors.id") - assert_queries_match(/WHERE #{Regexp.escape(author_id)} NOT IN \((@\d), \g<1>\)'/) do + assert_queries_match(/WHERE #{Regexp.escape(author_id)} NOT IN \((@\d), \g<1>\)/) do assert_equal [david], non_mary_and_bob.merge(non_mary_and_bob) end @@ -2420,7 +2355,7 @@ def test_preloads_has_many_on_model_with_a_composite_primary_key_through_id_attr c = Cpk::OrderAgreement.lease_connection order_id_column = Regexp.escape(c.quote_table_name("cpk_order_agreements.order_id")) - order_id_constraint = /#{order_id_column} = @0.*@0 = \d+$/ + order_id_constraint = /#{order_id_column} = @0$/ expectation = /SELECT.*WHERE.* #{order_id_constraint}/ assert_match(expectation, preload_sql) @@ -2444,7 +2379,7 @@ def test_preloads_belongs_to_a_composite_primary_key_model_through_id_attribute_ c = Cpk::Order.lease_connection order_id = Regexp.escape(c.quote_table_name("cpk_orders.id")) - order_constraint = /#{order_id} = @0.*@0 = \d+$/ + order_constraint = /#{order_id} = @0$/ expectation = /SELECT.*WHERE.* #{order_constraint}/ assert_match(expectation, preload_sql) @@ -2509,66 +2444,6 @@ def test_in_order_of_with_nil_coerced end class QueryLogsTest < ActiveRecord::TestCase - # SQL requires double single-quotes. - coerce_tests! :test_sql_commenter_format - def test_sql_commenter_format_coerced - ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter - ActiveRecord::QueryLogs.tags = [:application] - - assert_queries_match(%r{/\*application=''active_record''\*/}) do - Dashboard.first - end - end - - # SQL requires double single-quotes. - coerce_tests! :test_sqlcommenter_format_value - def test_sqlcommenter_format_value_coerced - ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter - - ActiveRecord::QueryLogs.tags = [ - :application, - {tracestate: "congo=t61rcWkgMzE,rojo=00f067aa0ba902b7", custom_proc: -> { "Joe's Shack" }} - ] - - assert_queries_match(%r{custom_proc=''Joe%27s%20Shack'',tracestate=''congo%3Dt61rcWkgMzE%2Crojo%3D00f067aa0ba902b7''\*/}) do - Dashboard.first - end - end - - # SQL requires double single-quotes. - coerce_tests! :test_sqlcommenter_format_value_string_coercible - def test_sqlcommenter_format_value_string_coercible_coerced - ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter - - ActiveRecord::QueryLogs.tags = [ - :application, - {custom_proc: -> { 1234 }} - ] - - assert_queries_match(%r{custom_proc=''1234''\*/}) do - Dashboard.first - end - end - - # SQL requires double single-quotes. - coerce_tests! :test_sqlcommenter_format_allows_string_keys - def test_sqlcommenter_format_allows_string_keys_coerced - ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter - - ActiveRecord::QueryLogs.tags = [ - :application, - { - "string" => "value", - :tracestate => "congo=t61rcWkgMzE,rojo=00f067aa0ba902b7", - :custom_proc => -> { "Joe's Shack" } - } - ] - - assert_queries_match(%r{custom_proc=''Joe%27s%20Shack'',string=''value'',tracestate=''congo%3Dt61rcWkgMzE%2Crojo%3D00f067aa0ba902b7''\*/}) do - Dashboard.first - end - end - # Invalid character encoding causes `ActiveRecord::StatementInvalid` error similar to Postgres. coerce_tests! :test_invalid_encoding_query def test_invalid_encoding_query_coerced @@ -2611,7 +2486,7 @@ def test_upsert_all_updates_using_provided_sql_coerced ELSE 1 END SQL - ) + ) ) assert_equal "published", Book.find(1).status @@ -2816,31 +2691,6 @@ def type_for_attribute_is_not_aware_of_custom_types_coerced end end -class ExplainTest < ActiveRecord::TestCase - # Expected query slightly different from because of 'sp_executesql' and query parameters. - coerce_tests! :test_relation_explain_with_first - def test_relation_explain_with_first_coerced - expected_query = capture_sql { - Car.all.first - }.first[/EXEC sp_executesql N'(.*?) NEXT/, 1] - message = Car.all.explain.first - assert_match(/^EXPLAIN/, message) - assert_match(expected_query, message) - end - - # Expected query slightly different from because of 'sp_executesql' and query parameters. - coerce_tests! :test_relation_explain_with_last - def test_relation_explain_with_last_coerced - expected_query = capture_sql { - Car.all.last - }.first[/EXEC sp_executesql N'(.*?) NEXT/, 1] - message = Car.all.explain.last - - assert_match(/^EXPLAIN/, message) - assert_match(expected_query, message) - end -end - module ActiveRecord module Assertions class QueryAssertionsTest < ActiveSupport::TestCase diff --git a/test/cases/optimizer_hints_test_sqlserver.rb b/test/cases/optimizer_hints_test_sqlserver.rb index b1aab6820..fde5e9144 100644 --- a/test/cases/optimizer_hints_test_sqlserver.rb +++ b/test/cases/optimizer_hints_test_sqlserver.rb @@ -29,7 +29,7 @@ class OptimizerHitsTestSQLServer < ActiveRecord::TestCase end it "support subqueries" do - assert_queries_match(%r{.*'SELECT COUNT\(count_column\) FROM \(SELECT .*\) subquery_for_count OPTION \(MAXDOP 2\)'.*}) do + assert_queries_match(%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 diff --git a/test/cases/showplan_test_sqlserver.rb b/test/cases/showplan_test_sqlserver.rb index 3bf9e1538..6dfde5386 100644 --- a/test/cases/showplan_test_sqlserver.rb +++ b/test/cases/showplan_test_sqlserver.rb @@ -28,13 +28,13 @@ class ShowplanTestSQLServer < ActiveRecord::TestCase it "from array condition using index" do plan = Car.where(id: [1, 2]).explain.inspect - _(plan).must_include "SELECT [cars].* FROM [cars] WHERE [cars].[id] IN (1, 2)" + _(plan).must_include "SELECT [cars].* FROM [cars] WHERE [cars].[id] IN (@0, @1)" _(plan).must_include "Clustered Index Seek", "make sure we do not showplan the sp_executesql" end it "from array condition" do plan = Car.where(name: ["honda", "zyke"]).explain.inspect - _(plan).must_include " SELECT [cars].* FROM [cars] WHERE [cars].[name] IN (N'honda', N'zyke')" + _(plan).must_include " SELECT [cars].* FROM [cars] WHERE [cars].[name] IN (@0, @1)" _(plan).must_include "Clustered Index Scan", "make sure we do not showplan the sp_executesql" end end diff --git a/test/cases/specific_schema_test_sqlserver.rb b/test/cases/specific_schema_test_sqlserver.rb index bfdce3617..0829012e5 100644 --- a/test/cases/specific_schema_test_sqlserver.rb +++ b/test/cases/specific_schema_test_sqlserver.rb @@ -116,16 +116,16 @@ def quoted_id end end # Using ActiveRecord's quoted_id feature for objects. - assert_queries_match(/@0 = 'T'/) { SSTestDatatypeMigration.where(char_col: value.new).first } - assert_queries_match(/@0 = 'T'/) { SSTestDatatypeMigration.where(varchar_col: value.new).first } + assert_queries_and_values_match(/.*/, ["'T'", 1]) { SSTestDatatypeMigration.where(char_col: value.new).first } + assert_queries_and_values_match(/.*/, ["'T'", 1]) { SSTestDatatypeMigration.where(varchar_col: value.new).first } # Using our custom char type data. type = ActiveRecord::Type::SQLServer::Char data = ActiveRecord::Type::SQLServer::Data - assert_queries_match(/@0 = 'T'/) { SSTestDatatypeMigration.where(char_col: data.new("T", type.new)).first } - assert_queries_match(/@0 = 'T'/) { SSTestDatatypeMigration.where(varchar_col: data.new("T", type.new)).first } + assert_queries_and_values_match(/.*/, ["'T'", 1]) { SSTestDatatypeMigration.where(char_col: data.new("T", type.new)).first } + assert_queries_and_values_match(/.*/, ["'T'", 1]) { SSTestDatatypeMigration.where(varchar_col: data.new("T", type.new)).first } # Taking care of everything. - assert_queries_match(/@0 = 'T'/) { SSTestDatatypeMigration.where(char_col: "T").first } - assert_queries_match(/@0 = 'T'/) { SSTestDatatypeMigration.where(varchar_col: "T").first } + assert_queries_and_values_match(/.*/, ["'T'", 1]) { SSTestDatatypeMigration.where(char_col: "T").first } + assert_queries_and_values_match(/.*/, ["'T'", 1]) { SSTestDatatypeMigration.where(varchar_col: "T").first } end it "can update and hence properly quoted non-national char/varchar columns" do diff --git a/test/support/query_assertions.rb b/test/support/query_assertions.rb index 2abd98159..46f2990d8 100644 --- a/test/support/query_assertions.rb +++ b/test/support/query_assertions.rb @@ -22,6 +22,28 @@ def assert_queries_count(count = nil, include_schema: false, &block) end end + def assert_queries_and_values_match(match, bound_values=[], count: nil, &block) + ActiveRecord::Base.lease_connection.materialize_transactions + + counter = ActiveRecord::Assertions::QueryAssertions::SQLCounter.new + ActiveSupport::Notifications.subscribed(counter, "sql.active_record") do + result = _assert_nothing_raised_or_warn("assert_queries_match", &block) + queries = counter.log_full + matched_queries = queries.select do |query, values| + values = values.map { |v| v.respond_to?(:quoted) ? v.quoted : v } + match === query && bound_values === values + end + + if count + assert_equal count, matched_queries.size, "#{matched_queries.size} instead of #{count} queries were executed.#{count.log.empty? ? '' : "\nQueries:\n#{counter.log.join("\n")}"}" + else + assert_operator matched_queries.size, :>=, 1, "1 or more queries expected, but none were executed.#{counter.log.empty? ? '' : "\nQueries:\n#{counter.log.join("\n")}"}" + end + + result + end + end + private # Rails tests expect a save-point to be created and released. SQL Server does not release From 785f2bab2e1eb0e6811ee84ab26858d3f2ba40a8 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 6 Nov 2025 20:39:56 +0000 Subject: [PATCH 2/3] Standardrb --- .../sqlserver/database_statements.rb | 90 +++++++++---------- test/cases/coerced_tests.rb | 31 +++---- test/support/query_assertions.rb | 6 +- 3 files changed, 64 insertions(+), 63 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index e7262774e..daf635d63 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -22,12 +22,12 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif id_insert_table_name = query_requires_identity_insert?(sql) result, affected_rows = if id_insert_table_name - with_identity_insert_enabled(id_insert_table_name, raw_connection) do - internal_exec_sql_query(sql, raw_connection) - end - else - internal_exec_sql_query(sql, raw_connection) - end + with_identity_insert_enabled(id_insert_table_name, raw_connection) do + internal_exec_sql_query(sql, raw_connection) + end + else + internal_exec_sql_query(sql, raw_connection) + end verified! notification_payload[:affected_rows] = affected_rows @@ -231,10 +231,10 @@ def merge_insert_values_list(insert:, insert_all:) def execute_procedure(proc_name, *variables) vars = if variables.any? && variables.first.is_a?(Hash) - variables.first.map { |k, v| "@#{k} = #{quote(v)}" } - else - variables.map { |v| quote(v) } - end.join(", ") + variables.first.map { |k, v| "@#{k} = #{quote(v)}" } + else + variables.map { |v| quote(v) } + end.join(", ") sql = "EXEC #{proc_name} #{vars}".strip log(sql, "Execute Procedure") do |notification_payload| @@ -340,35 +340,35 @@ def sql_for_insert(sql, pk, binds, returning) end sql = if pk && use_output_inserted? && !database_prefix_remote_server? - table_name ||= get_table_name(sql) - exclude_output_inserted = exclude_output_inserted_table_name?(table_name, sql) - - if exclude_output_inserted - pk_and_types = Array(pk).map do |subkey| - { - quoted: SQLServer::Utils.extract_identifiers(subkey).quoted, - id_sql_type: exclude_output_inserted_id_sql_type(subkey, exclude_output_inserted) - } - end - - <<~SQL.squish + table_name ||= get_table_name(sql) + exclude_output_inserted = exclude_output_inserted_table_name?(table_name, sql) + + if exclude_output_inserted + pk_and_types = Array(pk).map do |subkey| + { + quoted: SQLServer::Utils.extract_identifiers(subkey).quoted, + id_sql_type: exclude_output_inserted_id_sql_type(subkey, exclude_output_inserted) + } + end + + <<~SQL.squish DECLARE @ssaIdInsertTable table (#{pk_and_types.map { |pk_and_type| "#{pk_and_type[:quoted]} #{pk_and_type[:id_sql_type]}" }.join(", ")}); #{sql.dup.insert sql.index(/ (DEFAULT )?VALUES/i), " OUTPUT #{pk_and_types.map { |pk_and_type| "INSERTED.#{pk_and_type[:quoted]}" }.join(", ")} INTO @ssaIdInsertTable"} SELECT #{pk_and_types.map { |pk_and_type| "CAST(#{pk_and_type[:quoted]} AS #{pk_and_type[:id_sql_type]}) #{pk_and_type[:quoted]}" }.join(", ")} FROM @ssaIdInsertTable SQL - else - returning_columns = returning || Array(pk) - - if returning_columns.any? - returning_columns_statements = returning_columns.map { |c| " INSERTED.#{SQLServer::Utils.extract_identifiers(c).quoted}" } - sql.dup.insert sql.index(/ (DEFAULT )?VALUES/i), " OUTPUT" + returning_columns_statements.join(",") - else - sql - end - end - else - "#{sql}; SELECT CAST(SCOPE_IDENTITY() AS bigint) AS Ident" - end + else + returning_columns = returning || Array(pk) + + if returning_columns.any? + returning_columns_statements = returning_columns.map { |c| " INSERTED.#{SQLServer::Utils.extract_identifiers(c).quoted}" } + sql.dup.insert sql.index(/ (DEFAULT )?VALUES/i), " OUTPUT" + returning_columns_statements.join(",") + else + sql + end + end + else + "#{sql}; SELECT CAST(SCOPE_IDENTITY() AS bigint) AS Ident" + end [sql, binds] end @@ -537,16 +537,16 @@ def build_sql_for_returning(insert:, insert_all:) return "" unless insert_all.returning returning_values_sql = if insert_all.returning.is_a?(String) - insert_all.returning - else - Array(insert_all.returning).map do |attribute| - if insert.model.attribute_alias?(attribute) - "INSERTED.#{quote_column_name(insert.model.attribute_alias(attribute))} AS #{quote_column_name(attribute)}" - else - "INSERTED.#{quote_column_name(attribute)}" - end - end.join(",") - end + insert_all.returning + else + Array(insert_all.returning).map do |attribute| + if insert.model.attribute_alias?(attribute) + "INSERTED.#{quote_column_name(insert.model.attribute_alias(attribute))} AS #{quote_column_name(attribute)}" + else + "INSERTED.#{quote_column_name(attribute)}" + end + end.join(",") + end " OUTPUT #{returning_values_sql}" end diff --git a/test/cases/coerced_tests.rb b/test/cases/coerced_tests.rb index 6ac8d71a9..fa5076862 100644 --- a/test/cases/coerced_tests.rb +++ b/test/cases/coerced_tests.rb @@ -261,7 +261,7 @@ def test_belongs_to_with_primary_key_joins_on_correct_column_coerced def test_belongs_to_coerced client = Client.find(3) first_firm = companies(:first_firm) - assert_queries_and_values_match(/FETCH NEXT @3 ROWS ONLY/, ['Firm', 'Agency', 1, 1]) do + assert_queries_and_values_match(/FETCH NEXT @3 ROWS ONLY/, ["Firm", "Agency", 1, 1]) do assert_equal first_firm, client.firm assert_equal first_firm.name, client.firm.name end @@ -397,11 +397,11 @@ def test_select_avg_with_group_by_as_virtual_attribute_with_ar_coerced rails_core = companies(:rails_core) account = Account - .select(:firm_id, "AVG(CAST(credit_limit AS DECIMAL)) AS avg_credit_limit") - .where(firm: rails_core) - .group(:firm_id) - .order(:firm_id) - .take! + .select(:firm_id, "AVG(CAST(credit_limit AS DECIMAL)) AS avg_credit_limit") + .where(firm: rails_core) + .group(:firm_id) + .order(:firm_id) + .take! # id was not selected, so it should be nil # (cannot select id because it wasn't used in the GROUP BY clause) @@ -448,11 +448,11 @@ def test_select_avg_with_joins_and_group_by_as_virtual_attribute_with_ar_coerced rails_core = companies(:rails_core) firm = DependentFirm - .select("companies.*", "AVG(CAST(accounts.credit_limit AS DECIMAL)) AS avg_credit_limit") - .where(id: rails_core) - .joins(:account) - .group(:id, :type, :firm_id, :firm_name, :name, :client_of, :rating, :account_id, :description, :status) - .take! + .select("companies.*", "AVG(CAST(accounts.credit_limit AS DECIMAL)) AS avg_credit_limit") + .where(id: rails_core) + .joins(:account) + .group(:id, :type, :firm_id, :firm_name, :name, :client_of, :rating, :account_id, :description, :status) + .take! # all the DependentFirm attributes should be present assert_equal rails_core, firm @@ -997,7 +997,7 @@ class FinderTest < ActiveRecord::TestCase # We have implicit ordering, via FETCH. coerce_tests! %r{doesn't have implicit ordering}, - :test_find_doesnt_have_implicit_ordering + :test_find_doesnt_have_implicit_ordering # Assert SQL Server limit implementation coerce_tests! :test_take_and_first_and_last_with_integer_should_use_sql_limit @@ -1154,7 +1154,8 @@ def test_implicit_order_column_prepends_query_constraints_coerced quoted_color = Regexp.escape(c.quote_table_name("clothing_items.color")) quoted_descrption = Regexp.escape(c.quote_table_name("clothing_items.description")) - assert_queries_match(/ORDER BY #{quoted_descrption} ASC, #{quoted_type} ASC, #{quoted_color} ASC OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY/i) do assert_kind_of ClothingItem, ClothingItem.first + assert_queries_match(/ORDER BY #{quoted_descrption} ASC, #{quoted_type} ASC, #{quoted_color} ASC OFFSET 0 ROWS FETCH NEXT @(\d) ROWS ONLY/i) do + assert_kind_of ClothingItem, ClothingItem.first end ensure ClothingItem.implicit_order_column = nil @@ -1481,7 +1482,7 @@ def test_reorder_with_first_coerced def test_multiple_where_and_having_clauses_coerced post = Post.first having_then_where = Post.having(id: post.id).where(title: post.title) - .having(id: post.id).where(title: post.title).group(:id).select(:id) + .having(id: post.id).where(title: post.title).group(:id).select(:id) assert_equal [post], having_then_where end @@ -2486,7 +2487,7 @@ def test_upsert_all_updates_using_provided_sql_coerced ELSE 1 END SQL - ) + ) ) assert_equal "published", Book.find(1).status diff --git a/test/support/query_assertions.rb b/test/support/query_assertions.rb index 46f2990d8..78dfce02d 100644 --- a/test/support/query_assertions.rb +++ b/test/support/query_assertions.rb @@ -22,7 +22,7 @@ def assert_queries_count(count = nil, include_schema: false, &block) end end - def assert_queries_and_values_match(match, bound_values=[], count: nil, &block) + def assert_queries_and_values_match(match, bound_values = [], count: nil, &block) ActiveRecord::Base.lease_connection.materialize_transactions counter = ActiveRecord::Assertions::QueryAssertions::SQLCounter.new @@ -35,9 +35,9 @@ def assert_queries_and_values_match(match, bound_values=[], count: nil, &block) end if count - assert_equal count, matched_queries.size, "#{matched_queries.size} instead of #{count} queries were executed.#{count.log.empty? ? '' : "\nQueries:\n#{counter.log.join("\n")}"}" + assert_equal count, matched_queries.size, "#{matched_queries.size} instead of #{count} queries were executed.#{"\nQueries:\n#{counter.log.join("\n")}" unless count.log.empty?}" else - assert_operator matched_queries.size, :>=, 1, "1 or more queries expected, but none were executed.#{counter.log.empty? ? '' : "\nQueries:\n#{counter.log.join("\n")}"}" + assert_operator matched_queries.size, :>=, 1, "1 or more queries expected, but none were executed.#{"\nQueries:\n#{counter.log.join("\n")}" unless counter.log.empty?}" end result From e7d36c729c17182662f7d0a3fe3fe62ee251b624 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 6 Nov 2025 20:47:48 +0000 Subject: [PATCH 3/3] Update coerced_tests.rb --- test/cases/coerced_tests.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cases/coerced_tests.rb b/test/cases/coerced_tests.rb index fa5076862..0615ff254 100644 --- a/test/cases/coerced_tests.rb +++ b/test/cases/coerced_tests.rb @@ -1087,7 +1087,7 @@ def test_implicit_order_column_is_configurable_with_multiple_values_coerced old_implicit_order_column = Topic.implicit_order_column Topic.implicit_order_column = ["title", "author_name"] - assert_queries_match(/ORDER BY #{Regexp.escape(quote_table_name("topics.title"))} DESC, #{Regexp.escape(quote_table_name("topics.author_name"))} DESC, #{Regexp.escape(quote_table_name("topics.id"))} DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY.*@0 = 1/i) { + assert_queries_and_values_match(/ORDER BY #{Regexp.escape(quote_table_name("topics.title"))} DESC, #{Regexp.escape(quote_table_name("topics.author_name"))} DESC, #{Regexp.escape(quote_table_name("topics.id"))} DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY/i, [1]) { Topic.last } ensure @@ -1100,7 +1100,7 @@ def test_ordering_does_not_append_primary_keys_or_query_constraints_if_passed_an old_implicit_order_column = Topic.implicit_order_column Topic.implicit_order_column = ["author_name", nil] - assert_queries_match(/ORDER BY #{Regexp.escape(quote_table_name("topics.author_name"))} DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY.*@0 = 1/i) { + assert_queries_and_values_match(/ORDER BY #{Regexp.escape(quote_table_name("topics.author_name"))} DESC OFFSET 0 ROWS FETCH NEXT @0 ROWS ONLY/i, [1]) { Topic.last } ensure