diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index e7f83374c..9666c24d0 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -1,7 +1,7 @@ # See here for image contents: https://github.com/microsoft/vscode-dev-containers/tree/v0.191.1/containers/ruby/.devcontainer/base.Dockerfile # [Choice] Ruby version: 3, 3.0, 2, 2.7, 2.6 -ARG VARIANT="3" +ARG VARIANT="4" FROM mcr.microsoft.com/devcontainers/ruby:${VARIANT} # Update Yarn keys @@ -32,4 +32,3 @@ COPY Gemfile VERSION activerecord-sqlserver-adapter.gemspec /tmp/activerecord-sq RUN cd /tmp/activerecord-sqlserver-adapter \ && RAILS_BRANCH=main bundle install \ && rm -rf /tmp/activerecord-sqlserver-adapter -RUN chown -R vscode:vscode /usr/local/rvm diff --git a/CHANGELOG.md b/CHANGELOG.md index 35ede15d0..d07794bd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ #### Added - [#1385](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1385) Added support for Nulls first and last. +- [#1422](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1422) Added ractors support. #### Changed diff --git a/lib/active_record/connection_adapters/sqlserver/core_ext/preloader.rb b/lib/active_record/connection_adapters/sqlserver/core_ext/preloader.rb index d7bc7b9e4..7398b85a9 100644 --- a/lib/active_record/connection_adapters/sqlserver/core_ext/preloader.rb +++ b/lib/active_record/connection_adapters/sqlserver/core_ext/preloader.rb @@ -13,19 +13,19 @@ def load_records_for_keys(keys, &block) return [] if keys.empty? - if association_key_name.is_a?(Array) + if destination_key_name.is_a?(Array) query_constraints = Hash.new { |hsh, key| hsh[key] = Set.new } keys.each_with_object(query_constraints) do |values_set, constraints| - association_key_name.zip(values_set).each do |key_name, value| - constraints[key_name] << value + destination_key_name.each_with_index do |key_name, index| + constraints[key_name] << values_set[index] end end scope.where(query_constraints).load(&block) else keys.each_slice(in_clause_length).flat_map do |slice| - scope.where(association_key_name => slice).load(&block).records + scope.where(destination_key_name => slice).load(&block).records end end end diff --git a/lib/active_record/connection_adapters/sqlserver/ractor_proxy_adapter.rb b/lib/active_record/connection_adapters/sqlserver/ractor_proxy_adapter.rb new file mode 100644 index 000000000..3ae9a429b --- /dev/null +++ b/lib/active_record/connection_adapters/sqlserver/ractor_proxy_adapter.rb @@ -0,0 +1,365 @@ +# frozen_string_literal: true + +# :markup: markdown + +require "active_record/connection_adapters/ractor_connection_handler/abstract_proxy_adapter" + +module ActiveRecord + module ConnectionAdapters + class RactorConnectionHandler # :nodoc: + class SQLServerProxyAdapter < AbstractProxyAdapter # :nodoc: + def disable_referential_integrity + tables = tables_with_referential_integrity + tables.each { |t| execute "ALTER TABLE #{quote_table_name(t)} NOCHECK CONSTRAINT ALL" } + + begin + yield + ensure + tables.each { |t| execute "ALTER TABLE #{quote_table_name(t)} CHECK CONSTRAINT ALL" } + end + end + + def create_savepoint(name = current_savepoint_name) + query_command("SAVE TRANSACTION #{name}", "TRANSACTION") + end + + def exec_rollback_to_savepoint(name = current_savepoint_name) + query_command("ROLLBACK TRANSACTION #{name}", "TRANSACTION") + end + + def release_savepoint(_name = current_savepoint_name) + # SQL Server does not require savepoints to be explicitly released. + end + + # CAPABILITIES ============================================ + + def supports_advisory_locks? + pure_remote_dispatch(:supports_advisory_locks?) + end + + def supports_bulk_alter? + pure_remote_dispatch(:supports_bulk_alter?) + end + + def supports_check_constraints? + pure_remote_dispatch(:supports_check_constraints?) + end + + def supports_comments? + pure_remote_dispatch(:supports_comments?) + end + + def supports_comments_in_create? + pure_remote_dispatch(:supports_comments_in_create?) + end + + def supports_common_table_expressions? + pure_remote_dispatch(:supports_common_table_expressions?) + end + + def supports_ddl_transactions? + pure_remote_dispatch(:supports_ddl_transactions?) + end + + def supports_explain? + pure_remote_dispatch(:supports_explain?) + end + + def supports_expression_index? + pure_remote_dispatch(:supports_expression_index?) + end + + def supports_foreign_keys? + pure_remote_dispatch(:supports_foreign_keys?) + end + + def supports_index_include? + pure_remote_dispatch(:supports_index_include?) + end + + def supports_index_sort_order? + pure_remote_dispatch(:supports_index_sort_order?) + end + + def supports_indexes_in_create? + pure_remote_dispatch(:supports_indexes_in_create?) + end + + def supports_insert_conflict_target? + pure_remote_dispatch(:supports_insert_conflict_target?) + end + + def supports_insert_on_duplicate_skip? + pure_remote_dispatch(:supports_insert_on_duplicate_skip?) + end + + def supports_insert_on_duplicate_update? + pure_remote_dispatch(:supports_insert_on_duplicate_update?) + end + + def supports_insert_returning? + pure_remote_dispatch(:supports_insert_returning?) + end + + def supports_json? + pure_remote_dispatch(:supports_json?) + end + + def supports_lazy_transactions? + pure_remote_dispatch(:supports_lazy_transactions?) + end + + def supports_optimizer_hints? + pure_remote_dispatch(:supports_optimizer_hints?) + end + + def supports_partial_index? + pure_remote_dispatch(:supports_partial_index?) + end + + def supports_savepoints? + pure_remote_dispatch(:supports_savepoints?) + end + + def supports_transaction_isolation? + pure_remote_dispatch(:supports_transaction_isolation?) + end + + def supports_views? + pure_remote_dispatch(:supports_views?) + end + + def supports_virtual_columns? + pure_remote_dispatch(:supports_virtual_columns?) + end + + # QUOTING ================================================= + + def quote(...) + pure_remote_dispatch(:quote, ...) + end + + def quote_default_expression(...) + pure_remote_dispatch(:quote_default_expression, ...) + end + + def quote_string(...) + pure_remote_dispatch(:quote_string, ...) + end + + def quote_table_name_for_assignment(...) + pure_remote_dispatch(:quote_table_name_for_assignment, ...) + end + + def quoted_binary(...) + pure_remote_dispatch(:quoted_binary, ...) + end + + def quoted_date(...) + pure_remote_dispatch(:quoted_date, ...) + end + + def type_cast(...) + remote_dispatch(:type_cast, ...) + end + + def unquoted_false + remote_dispatch(:unquoted_false) + end + + def unquoted_true + remote_dispatch(:unquoted_true) + end + + # DATABASE STATEMENTS ===================================== + + def default_insert_value(...) + remote_dispatch(:default_insert_value, ...) + end + + def apply_returning_to!(intent, returning) + # SQL Server does not support the RETURNING syntax. Returning is + # handled on the main connection via OUTPUT / SCOPE_IDENTITY, so + # replicate that here on the worker instead of appending RETURNING. + return unless supports_insert_returning? + + intent.raw_sql = "#{intent.raw_sql}; SELECT CAST(SCOPE_IDENTITY() AS bigint) AS Ident" + end + + def high_precision_current_timestamp + remote_dispatch(:high_precision_current_timestamp) + end + + def write_query?(...) + remote_dispatch(:write_query?, ...) + end + + # SCHEMA STATEMENTS ======================================= + + def add_column(...) + remote_dispatch(:add_column, ...) + end + + def add_foreign_key(...) + remote_dispatch(:add_foreign_key, ...) + end + + def add_index(...) + remote_dispatch(:add_index, ...) + end + + def change_column(...) + remote_dispatch(:change_column, ...) + end + + def change_column_default(...) + remote_dispatch(:change_column_default, ...) + end + + def change_column_null(...) + remote_dispatch(:change_column_null, ...) + end + + def check_constraints(...) + remote_dispatch(:check_constraints, ...) + end + + def columns_for_distinct(...) + remote_dispatch(:columns_for_distinct, ...) + end + + def create_schema_dumper(options) + SQLServer::SchemaDumper.create(self, options) + end + + def default_sequence_name(...) + remote_dispatch(:default_sequence_name, ...) + end + + def drop_table(...) + remote_dispatch(:drop_table, ...) + end + + def foreign_keys(...) + remote_dispatch(:foreign_keys, ...) + end + + def index_name(...) + remote_dispatch(:index_name, ...) + end + + def index_name_exists?(...) + remote_dispatch(:index_name_exists?, ...) + end + + def remove_column(...) + remote_dispatch(:remove_column, ...) + end + + def remove_foreign_key(...) + remote_dispatch(:remove_foreign_key, ...) + end + + def remove_index(...) + remote_dispatch(:remove_index, ...) + end + + def rename_column(...) + remote_dispatch(:rename_column, ...) + end + + def rename_index(...) + remote_dispatch(:rename_index, ...) + end + + def rename_table(...) + remote_dispatch(:rename_table, ...) + end + + def table_comment(...) + remote_dispatch(:table_comment, ...) + end + + def type_to_sql(...) + pure_remote_dispatch(:type_to_sql, ...) + end + + def update_table_definition(...) + remote_dispatch(:update_table_definition, ...) + end + + # ADAPTER SPECIFIC ======================================== + + def get_database_version + remote_dispatch(:get_database_version) + end + + def quoted_scope(...) + pure_remote_dispatch(:quoted_scope, ...) + end + + private + + # SCHEMA STATEMENTS ======================================= + + def add_options_for_index_columns(...) + remote_dispatch(:add_options_for_index_columns, ...) + end + + def change_column_comment_sql(...) + remote_dispatch(:change_column_comment_sql, ...) + end + + def change_index_comment_sql(...) + remote_dispatch(:change_index_comment_sql, ...) + end + + def change_table_comment_sql(...) + remote_dispatch(:change_table_comment_sql, ...) + end + + def create_alter_table(...) + remote_dispatch(:create_alter_table, ...) + end + + def data_source_sql(...) + remote_dispatch(:data_source_sql, ...) + end + + def drop_table_sql(...) + remote_dispatch(:drop_table_sql, ...) + end + + def extract_foreign_key_action(...) + remote_dispatch(:extract_foreign_key_action, ...) + end + + def fetch_table_options(...) + remote_dispatch(:fetch_table_options, ...) + end + + def fetch_type_metadata(...) + remote_dispatch(:fetch_type_metadata, ...) + end + + def reference_name_for_table(...) + remote_dispatch(:reference_name_for_table, ...) + end + + def validate_table_length!(...) + remote_dispatch(:validate_table_length!, ...) + end + + # ADAPTER SPECIFIC ======================================== + + def can_perform_case_insensitive_comparison_for?(...) + pure_remote_dispatch(:can_perform_case_insensitive_comparison_for?, ...) + end + + def warning_ignored?(...) + remote_dispatch(:warning_ignored?, ...) + end + end + end + end +end diff --git a/lib/active_record/connection_adapters/sqlserver_adapter.rb b/lib/active_record/connection_adapters/sqlserver_adapter.rb index 4dc83b875..47f134260 100644 --- a/lib/active_record/connection_adapters/sqlserver_adapter.rb +++ b/lib/active_record/connection_adapters/sqlserver_adapter.rb @@ -25,6 +25,7 @@ require "active_record/connection_adapters/sqlserver/schema_statements" require "active_record/connection_adapters/sqlserver/sql_type_metadata" require "active_record/connection_adapters/sqlserver/showplan" +require "active_record/connection_adapters/sqlserver/ractor_proxy_adapter" require "active_record/connection_adapters/sqlserver/table_definition" require "active_record/connection_adapters/sqlserver/quoting" require "active_record/connection_adapters/sqlserver/utils" @@ -136,6 +137,10 @@ def rails_application_name def native_database_types # :nodoc: NATIVE_DATABASE_TYPES end + + def ractor_connection_proxy_class # :nodoc: + RactorConnectionHandler::SQLServerProxyAdapter + end end def initialize(...) @@ -273,6 +278,36 @@ def supports_virtual_columns? true end + def ractor_connection_capabilities # :nodoc: + super.merge( + supports_advisory_locks?: supports_advisory_locks?, + supports_bulk_alter?: supports_bulk_alter?, + supports_check_constraints?: supports_check_constraints?, + supports_comments?: supports_comments?, + supports_comments_in_create?: supports_comments_in_create?, + supports_common_table_expressions?: supports_common_table_expressions?, + supports_ddl_transactions?: supports_ddl_transactions?, + supports_explain?: supports_explain?, + supports_expression_index?: supports_expression_index?, + supports_foreign_keys?: supports_foreign_keys?, + supports_index_include?: supports_index_include?, + supports_index_sort_order?: supports_index_sort_order?, + supports_indexes_in_create?: supports_indexes_in_create?, + supports_insert_conflict_target?: supports_insert_conflict_target?, + supports_insert_on_duplicate_skip?: supports_insert_on_duplicate_skip?, + supports_insert_on_duplicate_update?: supports_insert_on_duplicate_update?, + supports_insert_returning?: supports_insert_returning?, + supports_json?: supports_json?, + supports_lazy_transactions?: supports_lazy_transactions?, + supports_optimizer_hints?: supports_optimizer_hints?, + supports_partial_index?: supports_partial_index?, + supports_savepoints?: supports_savepoints?, + supports_transaction_isolation?: supports_transaction_isolation?, + supports_views?: supports_views?, + supports_virtual_columns?: supports_virtual_columns? + ) + end + def return_value_after_insert?(column) # :nodoc: column.is_primary? || column.is_identity? end @@ -486,7 +521,7 @@ def type_map def translate_exception(exception, message:, sql:, binds:) case message - when /(SQL Server client is not connected)|(failed to execute statement)|(failed dbsqlsend)|(DBPROCESS is dead or not enabled)/i + when /(SQL Server client is not connected)|(failed to execute statement)|(failed dbsqlsend)|(DBPROCESS is dead or not enabled)|(closed connection)/i ConnectionNotEstablished.new(message, connection_pool: @pool) when /(cannot insert duplicate key .* with unique index) | (violation of (unique|primary) key constraint)/i RecordNotUnique.new(message, sql: sql, binds: binds, connection_pool: @pool) diff --git a/lib/arel/visitors/sqlserver.rb b/lib/arel/visitors/sqlserver.rb index fac75c736..b6b7d665d 100644 --- a/lib/arel/visitors/sqlserver.rb +++ b/lib/arel/visitors/sqlserver.rb @@ -13,7 +13,7 @@ class SQLServer < Arel::Visitors::ToSql # SQLServer ToSql/Visitor (Overrides) - BIND_BLOCK = proc { |i| "@#{i - 1}" } + BIND_BLOCK = ActiveSupport::Ractors.shareable_proc { |i| "@#{i - 1}" } private_constant :BIND_BLOCK def bind_block @@ -193,19 +193,15 @@ def visit_Arel_Nodes_OptimizerHints(o, collector) 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: - # github.com/rails-sqlserver/activerecord-sqlserver-adapter/issues/450 + # Quoting is always dispatched to the pinned connection, so never check + # out a connection here: doing so would check out a second connection + # when this visitor runs inside a worker Ractor. table_name = - begin - o.class.engine.with_connection do |connection| - if connection.respond_to?(:sqlserver?) && connection.database_prefix_remote_server? - remote_server_table_name(o) - else - quote_table_name(o.name) - end - end - rescue + if @connection.database_prefix_remote_server? + ActiveRecord::ConnectionAdapters::SQLServer::Utils.extract_identifiers( + "#{@connection.database_prefix}#{o.name}" + ).quoted + else quote_table_name(o.name) end @@ -409,14 +405,6 @@ def primary_Key_From_Table(t) column_name ? t[column_name] : nil end - def remote_server_table_name(o) - o.class.engine.with_connection do |connection| - ActiveRecord::ConnectionAdapters::SQLServer::Utils.extract_identifiers( - "#{connection.database_prefix}#{o.name}" - ).quoted - end - end - # Need to remove ordering from sub-queries unless TOP/OFFSET also used. Otherwise, SQLServer # returns error "The ORDER BY clause is invalid in views, inline functions, derived tables, # sub-queries, and common table expressions, unless TOP, OFFSET or FOR XML is also specified." diff --git a/test/cases/coerced_tests.rb b/test/cases/coerced_tests.rb index 2bdf483e7..00bd8d0c3 100644 --- a/test/cases/coerced_tests.rb +++ b/test/cases/coerced_tests.rb @@ -2502,101 +2502,13 @@ class MultiDbMigratorTest < ActiveRecord::TestCase end class FieldOrderedValuesTest < ActiveRecord::TestCase - # Need to remove index as SQL Server considers NULLs on a unique-index to be equal unlike PostgreSQL/MySQL/SQLite. - coerce_tests! :test_in_order_of_with_enums_values - def test_in_order_of_with_enums_values_coerced - Book.lease_connection.remove_index(:books, column: [:author_id, :name]) - - original_test_in_order_of_with_enums_values - ensure - Book.where(author_id: nil, name: nil).delete_all - Book.lease_connection.add_index(:books, [:author_id, :name], unique: true) - end - - # Need to remove index as SQL Server considers NULLs on a unique-index to be equal unlike PostgreSQL/MySQL/SQLite. - coerce_tests! :test_in_order_of_with_string_column - def test_in_order_of_with_string_column_coerced + def setup + super Book.lease_connection.remove_index(:books, column: [:author_id, :name]) - - original_test_in_order_of_with_string_column - ensure - Book.where(author_id: nil, name: nil).delete_all - Book.lease_connection.add_index(:books, [:author_id, :name], unique: true) end - # Need to remove index as SQL Server considers NULLs on a unique-index to be equal unlike PostgreSQL/MySQL/SQLite. - coerce_tests! :test_in_order_of_with_enums_keys - def test_in_order_of_with_enums_keys_coerced - Book.lease_connection.remove_index(:books, column: [:author_id, :name]) - - original_test_in_order_of_with_enums_keys - ensure - Book.where(author_id: nil, name: nil).delete_all - Book.lease_connection.add_index(:books, [:author_id, :name], unique: true) - end - - # Need to remove index as SQL Server considers NULLs on a unique-index to be equal unlike PostgreSQL/MySQL/SQLite. - coerce_tests! :test_in_order_of_with_nil - def test_in_order_of_with_nil_coerced - Book.lease_connection.remove_index(:books, column: [:author_id, :name]) - - original_test_in_order_of_with_nil - ensure - Book.where(author_id: nil, name: nil).delete_all - Book.lease_connection.add_index(:books, [:author_id, :name], unique: true) - end - - # Need to remove index as SQL Server considers NULLs on a unique-index to be equal unlike PostgreSQL/MySQL/SQLite. - coerce_tests! :test_in_order_of_with_array_values_with_nil - def test_in_order_of_with_array_values_with_nil_coerced - Book.lease_connection.remove_index(:books, column: [:author_id, :name]) - - original_test_in_order_of_with_array_values_with_nil - ensure - Book.where(author_id: nil, name: nil).delete_all - Book.lease_connection.add_index(:books, [:author_id, :name], unique: true) - end - - # Need to remove index as SQL Server considers NULLs on a unique-index to be equal unlike PostgreSQL/MySQL/SQLite. - coerce_tests! :test_in_order_of_with_out_of_bound_integer_does_not_match_nulls - def test_in_order_of_with_out_of_bound_integer_does_not_match_nulls_coerced - Book.lease_connection.remove_index(:books, column: [:author_id, :name]) - - original_test_in_order_of_with_out_of_bound_integer_does_not_match_nulls - ensure - Book.where(author_id: nil, name: nil).delete_all - Book.lease_connection.add_index(:books, [:author_id, :name], unique: true) - end - - # Need to remove index as SQL Server considers NULLs on a unique-index to be equal unlike PostgreSQL/MySQL/SQLite. - coerce_tests! :test_in_order_of_with_only_unrepresentable_values_does_not_build_empty_case - def test_in_order_of_with_only_unrepresentable_values_does_not_build_empty_case_coerced - Book.lease_connection.remove_index(:books, column: [:author_id, :name]) - - original_test_in_order_of_with_only_unrepresentable_values_does_not_build_empty_case - ensure - Book.where(author_id: nil, name: nil).delete_all - Book.lease_connection.add_index(:books, [:author_id, :name], unique: true) - end - - # Need to remove index as SQL Server considers NULLs on a unique-index to be equal unlike PostgreSQL/MySQL/SQLite. - coerce_tests! :test_in_order_of_with_unknown_enum_key_does_not_match_nulls - def test_in_order_of_with_unknown_enum_key_does_not_match_nulls_coerced - Book.lease_connection.remove_index(:books, column: [:author_id, :name]) - - original_test_in_order_of_with_unknown_enum_key_does_not_match_nulls - ensure - Book.where(author_id: nil, name: nil).delete_all - Book.lease_connection.add_index(:books, [:author_id, :name], unique: true) - end - - # Need to remove index as SQL Server considers NULLs on a unique-index to be equal unlike PostgreSQL/MySQL/SQLite. - coerce_tests! :test_in_order_of_with_column_from_cte - def test_in_order_of_with_column_from_cte_coerced - connection.remove_index(:books, column: [:author_id, :name]) - - original_test_in_order_of_with_column_from_cte - ensure + def teardown + super Book.where(author_id: nil, name: nil).delete_all Book.lease_connection.add_index(:books, [:author_id, :name], unique: true) end @@ -3041,3 +2953,30 @@ def fixtures_for_json_columns_get_encrypted_automatically send(:"original_fixtures for json columns get encrypted automatically") end end + +class ActiveRecord::ConnectionAdapters::RactorConnectionHandlingTest::ProxyAdapterTest < ActiveRecord::ConnectionAdapters::RactorConnectionHandlingTest + # SQL Server adapter converts "FOR UPDATE" locks to "WITH(UPDLOCK)". + coerce_tests! :test_arel_compilation_uses_token_pinned_connection + def test_arel_compilation_uses_token_pinned_connection_coerced + conn = proxy_connection + table = Arel::Table.new(name: widgets_table) + manager = table.project(table[:id]).lock(Arel.sql("FOR UPDATE")) + + sql = conn.to_sql(manager) + + assert_match(/WITH\(UPDLOCK\)/, sql) + + assert_equal 1, main_pool.connections.size + end +end + + +class ActiveRecord::ConnectionAdapters::RactorConnectionHandlingTest::ProxyAdapterTest < ActiveRecord::ConnectionAdapters::RactorConnectionHandlingTest + if !in_memory_db? && RUBY_VERSION >= "4.0" + undef_method :bind_placeholder + + def bind_placeholder(position) + "@#{position - 1}" + end + end +end