Merge branch 'main' into pk_autopopulated_trigger

Author: aidanharan

.devcontainer/Dockerfile

@@ -6,9 +6,9 @@ FROM mcr.microsoft.com/devcontainers/ruby:${VARIANT}
 
 # TinyTDS
 RUN apt-get -y install libc6-dev \
-    && wget http://www.freetds.org/files/stable/freetds-1.1.32.tar.gz \
-    && tar -xzf freetds-1.1.32.tar.gz \
-    && cd freetds-1.1.32 \
+    && wget http://www.freetds.org/files/stable/freetds-1.4.14.tar.gz \
+    && tar -xzf freetds-1.4.14.tar.gz \
+    && cd freetds-1.4.14 \
     && ./configure --prefix=/usr/local --with-tdsver=7.3 \
     && make \
     && make install

.github/workflows/ci.yml

@@ -14,9 +14,9 @@ jobs:
       fail-fast: false
       matrix:
         ruby:
-          - 3.1.4
-          - 3.2.2
-          - 3.3.0
+          - 3.1.6
+          - 3.2.4
+          - 3.3.2
 
     steps:
       - name: Checkout code

Dockerfile.ci

@@ -9,6 +9,6 @@ WORKDIR $WORKDIR
 
 COPY . $WORKDIR
 
-RUN RAILS_BRANCH=7-2-stable bundle install --jobs `expr $(cat /proc/cpuinfo | grep -c "cpu cores") - 1` --retry 3
+RUN bundle install --jobs `expr $(cat /proc/cpuinfo | grep -c "cpu cores") - 1` --retry 3
 
 CMD ["sh"]

Gemfile

@@ -8,7 +8,7 @@ gemspec
 
 gem "bcrypt"
 gem "pg",      ">= 0.18.0"
-gem "sqlite3", "~> 1.4"
+gem "sqlite3", ">= 1.6.6"
 gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw, :jruby]
 gem "benchmark-ips"
 gem "minitest", ">= 5.15.0"

VERSION

@@ -1 +1 @@
-7.2.0.alpha
+7.2.0.beta2

activerecord-sqlserver-adapter.gemspec

@@ -27,6 +27,6 @@ Gem::Specification.new do |spec|
   spec.test_files    = spec.files.grep(%r{^(test|spec|features)/})
   spec.require_paths = ["lib"]
 
-  spec.add_dependency "activerecord", "~> 7.2.0.alpha"
+  spec.add_dependency "activerecord", "~> 7.2.0.beta2"
   spec.add_dependency "tiny_tds"
 end

docker-compose.ci.yml

@@ -5,7 +5,6 @@ services:
   ci:
     environment:
       - ACTIVERECORD_UNITTEST_HOST=sqlserver
-      - RAILS_BRANCH=7-2-stable
     build:
       context: .
       dockerfile: Dockerfile.ci

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 :with_connection, :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

lib/arel/visitors/sqlserver.rb

@@ -129,10 +129,12 @@ def visit_Arel_Table(o, collector)
         # 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
-              quote_table_name(o.name)
+            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 Exception
             quote_table_name(o.name)
@@ -201,6 +203,11 @@ def collect_optimizer_hints(o, collector)
         collector
       end
 
+      def visit_Arel_Nodes_WithRecursive(o, collector)
+        collector << "WITH "
+        collect_ctes(o.children, collector)
+      end
+
       # SQLServer ToSql/Visitor (Additions)
 
       def visit_Arel_Nodes_SelectStatement_SQLServer_Lock(collector, options = {})
@@ -315,14 +322,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
 

test/cases/coerced_tests.rb

@@ -2735,3 +2735,26 @@ def test_assert_queries_match_coerced
     end
   end
 end
+
+module ActiveRecord
+  class WithTest < ActiveRecord::TestCase
+    # SQL contains just 'WITH' instead of 'WITH RECURSIVE' as expected by the original test.
+    coerce_tests! :test_with_recursive
+    def test_with_recursive_coerced
+      top_companies = Company.where(firm_id: nil).to_a
+      child_companies = Company.where(firm_id: top_companies).to_a
+      top_companies_and_children = (top_companies.map(&:id) + child_companies.map(&:id)).sort
+
+      relation = Company.with_recursive(
+        top_companies_and_children: [
+          Company.where(firm_id: nil),
+          Company.joins("JOIN top_companies_and_children ON companies.firm_id = top_companies_and_children.id"),
+        ]
+      ).from("top_companies_and_children AS companies")
+
+      assert_equal top_companies_and_children, relation.order(:id).pluck(:id)
+      assert_match "WITH ", relation.to_sql
+    end
+  end
+end
+

test/cases/rake_test_sqlserver.rb

@@ -138,18 +138,22 @@ class SQLServerRakeStructureDumpLoadTest < SQLServerRakeTest
 
   it "dumps structure and accounts for defncopy oddities" do
     skip "debug defncopy on windows later" if host_windows?
+
     quietly { db_tasks.structure_dump configuration, filename }
+
     _(filedata).wont_match %r{\AUSE.*\z}
     _(filedata).wont_match %r{\AGO.*\z}
-    _(filedata).must_match %r{email\s+nvarchar\(4000\)}
-    _(filedata).must_match %r{background1\s+nvarchar\(max\)}
-    _(filedata).must_match %r{background2\s+text\s+}
+    _(filedata).must_match %r{\[email\]\s+nvarchar\(4000\)}
+    _(filedata).must_match %r{\[background1\]\s+nvarchar\(max\)}
+    _(filedata).must_match %r{\[background2\]\s+text\s+}
   end
 
   it "can load dumped structure" do
     skip "debug defncopy on windows later" if host_windows?
+
     quietly { db_tasks.structure_dump configuration, filename }
-    _(filedata).must_match %r{CREATE TABLE dbo\.users}
+
+    _(filedata).must_match %r{CREATE TABLE \[dbo\]\.\[users\]}
     db_tasks.purge(configuration)
     _(connection.tables).wont_include "users"
     db_tasks.load_schema db_config, :sql, filename