From 6b2689d66542e8dc2e332e7ef4b91469ec6f601a Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 23 May 2024 18:50:09 +0100 Subject: [PATCH 1/4] Eliminate missed lease_connection calls --- .../tasks/sqlserver_database_tasks.rb | 70 ++++++++++--------- lib/arel/visitors/sqlserver.rb | 39 ++++++----- 2 files changed, 60 insertions(+), 49 deletions(-) diff --git a/lib/active_record/tasks/sqlserver_database_tasks.rb b/lib/active_record/tasks/sqlserver_database_tasks.rb index a33ebb83d..c27371d59 100644 --- a/lib/active_record/tasks/sqlserver_database_tasks.rb +++ b/lib/active_record/tasks/sqlserver_database_tasks.rb @@ -10,7 +10,7 @@ module Tasks class SQLServerDatabaseTasks DEFAULT_COLLATION = "SQL_Latin1_General_CP1_CI_AS" - delegate :lease_connection, :establish_connection, to: ActiveRecord::Base + delegate :establish_connection, to: ActiveRecord::Base def self.using_database_configurations? true @@ -23,8 +23,10 @@ def initialize(configuration) def create(master_established = false) establish_master_connection unless master_established - lease_connection.create_database configuration.database, configuration_hash.merge(collation: default_collation) - establish_connection configuration + with_connection do |connection| + connection.create_database(configuration.database, configuration_hash.merge(collation: default_collation)) + end + establish_connection(configuration) rescue ActiveRecord::StatementInvalid => e if /database .* already exists/i === e.message raise DatabaseAlreadyExists @@ -35,15 +37,15 @@ def create(master_established = false) def drop establish_master_connection - lease_connection.drop_database configuration.database + with_connection { |connection| connection.drop_database(configuration.database) } end def charset - lease_connection.charset + with_connection { |connection| connection.charset } end def collation - lease_connection.collation + with_connection { |connection| connection.collation } end def purge @@ -56,34 +58,38 @@ def clear_active_connections! ActiveRecord::Base.connection_handler.clear_active_connections!(:all) end - def structure_dump(filename, extra_flags) - server_arg = "-S #{Shellwords.escape(configuration_hash[:host])}" - server_arg += ":#{Shellwords.escape(configuration_hash[:port])}" if configuration_hash[:port] - command = [ - "defncopy-ttds", - server_arg, - "-D #{Shellwords.escape(configuration_hash[:database])}", - "-U #{Shellwords.escape(configuration_hash[:username])}", - "-P #{Shellwords.escape(configuration_hash[:password])}", - "-o #{Shellwords.escape(filename)}", - ] - table_args = lease_connection.tables.map { |t| Shellwords.escape(t) } - command.concat(table_args) - view_args = lease_connection.views.map { |v| Shellwords.escape(v) } - command.concat(view_args) - raise "Error dumping database" unless Kernel.system(command.join(" ")) - - dump = File.read(filename) - dump.gsub!(/^USE .*$\nGO\n/, "") # Strip db USE statements - dump.gsub!(/^GO\n/, "") # Strip db GO statements - dump.gsub!(/nvarchar\(8000\)/, "nvarchar(4000)") # Fix nvarchar(8000) column defs - dump.gsub!(/nvarchar\(-1\)/, "nvarchar(max)") # Fix nvarchar(-1) column defs - dump.gsub!(/text\(\d+\)/, "text") # Fix text(16) column defs - File.open(filename, "w") { |file| file.puts dump } + def structure_dump(filename, _extra_flags) + with_connection do |connection| + server_arg = "-S #{Shellwords.escape(configuration_hash[:host])}" + server_arg += ":#{Shellwords.escape(configuration_hash[:port])}" if configuration_hash[:port] + command = [ + "defncopy-ttds", + server_arg, + "-D #{Shellwords.escape(configuration_hash[:database])}", + "-U #{Shellwords.escape(configuration_hash[:username])}", + "-P #{Shellwords.escape(configuration_hash[:password])}", + "-o #{Shellwords.escape(filename)}", + ] + table_args = connection.tables.map { |t| Shellwords.escape(t) } + command.concat(table_args) + view_args = connection.views.map { |v| Shellwords.escape(v) } + command.concat(view_args) + raise "Error dumping database" unless Kernel.system(command.join(" ")) + + dump = File.read(filename) + dump.gsub!(/^USE .*$\nGO\n/, "") # Strip db USE statements + dump.gsub!(/^GO\n/, "") # Strip db GO statements + dump.gsub!(/nvarchar\(8000\)/, "nvarchar(4000)") # Fix nvarchar(8000) column defs + dump.gsub!(/nvarchar\(-1\)/, "nvarchar(max)") # Fix nvarchar(-1) column defs + dump.gsub!(/text\(\d+\)/, "text") # Fix text(16) column defs + File.open(filename, "w") { |file| file.puts dump } + end end - def structure_load(filename, extra_flags) - lease_connection.execute File.read(filename) + def structure_load(filename, _extra_flags) + with_connection do |connection| + connection.execute File.read(filename) + end end private diff --git a/lib/arel/visitors/sqlserver.rb b/lib/arel/visitors/sqlserver.rb index 9016522cb..e4beceb62 100644 --- a/lib/arel/visitors/sqlserver.rb +++ b/lib/arel/visitors/sqlserver.rb @@ -127,21 +127,24 @@ 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 - table_name = - begin - if o.class.engine.lease_connection.respond_to?(:sqlserver?) && o.class.engine.lease_connection.database_prefix_remote_server? - remote_server_table_name(o) - else + + o.class.engine.with_connection do |connection| + table_name = + begin + if connection.respond_to?(:sqlserver?) && connection.database_prefix_remote_server? + remote_server_table_name(o) + else + quote_table_name(o.name) + end + rescue Exception quote_table_name(o.name) end - rescue Exception - quote_table_name(o.name) - end - if o.table_alias - collector << "#{table_name} #{quote_table_name o.table_alias}" - else - collector << table_name + if o.table_alias + collector << "#{table_name} #{quote_table_name o.table_alias}" + else + collector << table_name + end end end @@ -315,14 +318,16 @@ def primary_Key_From_Table(t) end def remote_server_table_name(o) - ActiveRecord::ConnectionAdapters::SQLServer::Utils.extract_identifiers( - "#{o.class.engine.lease_connection.database_prefix}#{o.name}" - ).quoted + 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 subqueries unless TOP/OFFSET also used. Otherwise, SQLServer + # 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, - # subqueries, and common table expressions, unless TOP, OFFSET or FOR XML is also specified." + # sub-queries, and common table expressions, unless TOP, OFFSET or FOR XML is also specified." def remove_invalid_ordering_from_select_statement(node) return unless Arel::Nodes::SelectStatement === node From 57cf215ee086bd7f2d0e235a3fe868fbcdfd153e Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 23 May 2024 19:09:43 +0100 Subject: [PATCH 2/4] Update sqlserver_database_tasks.rb --- lib/active_record/tasks/sqlserver_database_tasks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_record/tasks/sqlserver_database_tasks.rb b/lib/active_record/tasks/sqlserver_database_tasks.rb index c27371d59..bbae97e7c 100644 --- a/lib/active_record/tasks/sqlserver_database_tasks.rb +++ b/lib/active_record/tasks/sqlserver_database_tasks.rb @@ -10,7 +10,7 @@ module Tasks class SQLServerDatabaseTasks DEFAULT_COLLATION = "SQL_Latin1_General_CP1_CI_AS" - delegate :establish_connection, to: ActiveRecord::Base + delegate :with_connection, :establish_connection, to: ActiveRecord::Base def self.using_database_configurations? true From fbdc3c02e42b229045c33e94ae056b9c1026c8e0 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 23 May 2024 19:20:49 +0100 Subject: [PATCH 3/4] Update sqlserver.rb --- lib/arel/visitors/sqlserver.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/arel/visitors/sqlserver.rb b/lib/arel/visitors/sqlserver.rb index e4beceb62..1ab75351c 100644 --- a/lib/arel/visitors/sqlserver.rb +++ b/lib/arel/visitors/sqlserver.rb @@ -127,10 +127,9 @@ 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 - - o.class.engine.with_connection do |connection| - table_name = - begin + 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 @@ -139,12 +138,12 @@ def visit_Arel_Table(o, collector) rescue Exception quote_table_name(o.name) end - - if o.table_alias - collector << "#{table_name} #{quote_table_name o.table_alias}" - else - collector << table_name end + + if o.table_alias + collector << "#{table_name} #{quote_table_name o.table_alias}" + else + collector << table_name end end From f0f7e70ec2f8bc3d0bfc03a12b2494e0fc96abf3 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 23 May 2024 19:28:13 +0100 Subject: [PATCH 4/4] Update sqlserver.rb --- lib/arel/visitors/sqlserver.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/arel/visitors/sqlserver.rb b/lib/arel/visitors/sqlserver.rb index 1ab75351c..8392eb102 100644 --- a/lib/arel/visitors/sqlserver.rb +++ b/lib/arel/visitors/sqlserver.rb @@ -135,9 +135,9 @@ def visit_Arel_Table(o, collector) else quote_table_name(o.name) end - rescue Exception - quote_table_name(o.name) end + rescue Exception + quote_table_name(o.name) end if o.table_alias