diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 687823942..3c48eb342 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,6 @@ jobs: fail-fast: false matrix: ruby: - - 2.7.7 - 3.1.3 - 3.2.1 diff --git a/.gitignore b/.gitignore index 27b05d95f..6365e37d2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ nbproject/ -debug.log +debug.log* .DS_Store pkg/ doc/ @@ -16,3 +16,5 @@ coverage/* .flooignore .floo .byebug_history +tmp/* +test/storage/test.sqlite3* diff --git a/CHANGELOG.md b/CHANGELOG.md index 647901c7b..e42a02da1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,39 @@ +## v7.0.7 + +#### Changed + +- [#1200](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1200) Remove ActiveRecord::Relation#calculate patch + +## v7.0.6 + +#### Added + +- [#1141](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1141) Added support for check constraints. + +## v7.0.5.1 + +#### Fixed + +- [#1133](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1133) Fix matching view's real column name + +## v7.0.5.0 + +#### Fixed + +- [#1113](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1113) Fix issue with default view value not being found because of case sensitivity +- [#1126](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1113) Fix view issue with default column value not found + +## v7.0.4.0 + +#### Changed + +- [#1073](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1073) Improve performance of view default function lookup + +#### Fixed + +- [#1088](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1088) Fix creation of stored procedures that contain insert statements +- [#1089](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1089) When changing columns set date-time columns to datetime(6) by default + ## v7.0.3.0 #### Fixed diff --git a/README.md b/README.md index cf412b10e..bdec83b8e 100644 --- a/README.md +++ b/README.md @@ -12,11 +12,11 @@ The SQL Server adapter for ActiveRecord using SQL Server 2012 or higher. Interested in older versions? We follow a rational versioning policy that tracks Rails. That means that our 7.x version of the adapter is only for the latest 7.x version of Rails. If you need the adapter for SQL Server 2008 or 2005, you are still in the right spot. Just install the latest 3.2.x to 4.1.x version of the adapter that matches your Rails version. We also have stable branches for each major/minor release of ActiveRecord. | Adapter Version | Rails Version | Support | -|-----------------| ------------- | ------------------------------------------------------------------------------------------- | -| `7.0.3.0` | `7.0.x` | [active](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/tree/main) | +|-----------------| ------------- |---------------------------------------------------------------------------------------------| +| `7.0.7` | `7.0.x` | [active](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/tree/7-0-stable) | | `6.1.2.1` | `6.1.x` | [active](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/tree/6-1-stable) | | `6.0.3` | `6.0.x` | [active](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/tree/6-0-stable) | -| `5.2.1` | `5.2.x` | [ended](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/tree/5-2-stable) | +| `5.2.1` | `5.2.x` | [ended](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/tree/5-2-stable) | | `5.1.6` | `5.1.x` | [ended](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/tree/5-1-stable) | | `4.2.18` | `4.2.x` | [ended](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/tree/4-2-stable) | | `4.1.8` | `4.1.x` | [ended](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/tree/4-1-stable) | diff --git a/VERSION b/VERSION index 8117d37ee..2f2974f9b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.0.3.0 +7.0.7 diff --git a/lib/active_record/connection_adapters/sqlserver/core_ext/calculations.rb b/lib/active_record/connection_adapters/sqlserver/core_ext/calculations.rb index da0e37f80..0346a090a 100644 --- a/lib/active_record/connection_adapters/sqlserver/core_ext/calculations.rb +++ b/lib/active_record/connection_adapters/sqlserver/core_ext/calculations.rb @@ -8,25 +8,6 @@ module ConnectionAdapters module SQLServer module CoreExt module Calculations - # Same as original except we don't perform PostgreSQL hack that removes ordering. - def calculate(operation, column_name) - return super unless klass.connection.adapter_name == "SQLServer" - - if has_include?(column_name) - relation = apply_join_dependency - - if operation.to_s.downcase == "count" - unless distinct_value || distinct_select?(column_name || select_for_count) - relation.distinct! - relation.select_values = [klass.primary_key || table[Arel.star]] - end - end - - relation.calculate(operation, column_name) - else - perform_calculation(operation, column_name) - end - end private diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index 5e154b6b2..522d9fac2 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -406,7 +406,7 @@ def query_requires_identity_insert?(sql) end def insert_sql?(sql) - !(sql =~ /^\s*(INSERT|EXEC sp_executesql N'INSERT)/i).nil? + !(sql =~ /\A\s*(INSERT|EXEC sp_executesql N'INSERT)/i).nil? end def identity_columns(table_name) diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index 34b112392..cba49dcd5 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -143,6 +143,15 @@ def remove_column(table_name, column_name, type = nil, **options) def change_column(table_name, column_name, type, options = {}) sql_commands = [] indexes = [] + + if type == :datetime + # If no precision then default it to 6. + options[:precision] = 6 unless options.key?(:precision) + + # If there is precision then column must be of type 'datetime2'. + type = :datetime2 unless options[:precision].nil? + end + column_object = schema_cache.columns(table_name).find { |c| c.name.to_s == column_name.to_s } without_constraints = options.key?(:default) || options.key?(:limit) default = if !options.key?(:default) && column_object @@ -150,24 +159,29 @@ def change_column(table_name, column_name, type, options = {}) else options[:default] end + if without_constraints || (column_object && column_object.type != type.to_sym) remove_default_constraint(table_name, column_name) indexes = indexes(table_name).select { |index| index.columns.include?(column_name.to_s) } remove_indexes(table_name, column_name) end + sql_commands << "UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote_default_expression(options[:default], column_object)} WHERE #{quote_column_name(column_name)} IS NULL" if !options[:null].nil? && options[:null] == false && !options[:default].nil? alter_command = "ALTER TABLE #{quote_table_name(table_name)} ALTER COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, limit: options[:limit], precision: options[:precision], scale: options[:scale])}" alter_command += " COLLATE #{options[:collation]}" if options[:collation].present? alter_command += " NOT NULL" if !options[:null].nil? && options[:null] == false sql_commands << alter_command + if without_constraints default = quote_default_expression(default, column_object || column_for(table_name, column_name)) sql_commands << "ALTER TABLE #{quote_table_name(table_name)} ADD CONSTRAINT #{default_constraint_name(table_name, column_name)} DEFAULT #{default} FOR #{quote_column_name(column_name)}" end + # Add any removed indexes back indexes.each do |index| sql_commands << "CREATE INDEX #{quote_table_name(index.name)} ON #{quote_table_name(table_name)} (#{index.columns.map { |c| quote_column_name(c) }.join(', ')})" end + sql_commands.each { |c| do_execute(c) } clear_cache! end @@ -226,9 +240,33 @@ def extract_foreign_key_action(action, fk_name) end end + def check_constraints(table_name) + sql = <<~SQL + select chk.name AS 'name', + chk.definition AS 'expression' + from sys.check_constraints chk + inner join sys.tables st on chk.parent_object_id = st.object_id + where + st.name = '#{table_name}' + SQL + + chk_info = exec_query(sql, "SCHEMA") + + chk_info.map do |row| + options = { + name: row["name"] + } + expression = row["expression"] + expression = expression[1..-2] if expression.start_with?("(") && expression.end_with?(")") + + CheckConstraintDefinition.new(table_name, expression, options) + end + end + def type_to_sql(type, limit: nil, precision: nil, scale: nil, **) type_limitable = %w(string integer float char nchar varchar nvarchar).include?(type.to_s) limit = nil unless type_limitable + case type.to_s when "integer" case limit @@ -371,6 +409,15 @@ def column_definitions(table_name) view_exists = view_exists?(table_name) view_tblnm = view_table_name(table_name) if view_exists + if view_exists + results = sp_executesql %{ + SELECT LOWER(c.COLUMN_NAME) AS [name], c.COLUMN_DEFAULT AS [default] + FROM #{database}.INFORMATION_SCHEMA.COLUMNS c + WHERE c.TABLE_NAME = #{quote(view_tblnm)} + }.squish, "SCHEMA", [] + default_functions = results.each.with_object({}) {|row, out| out[row["name"]] = row["default"] }.compact + end + sql = column_definitions_sql(database, identifier) binds = [] @@ -402,13 +449,8 @@ def column_definitions(table_name) ci[:default_function] = begin default = ci[:default_value] if default.nil? && view_exists - default = select_value %{ - SELECT c.COLUMN_DEFAULT - FROM #{database}.INFORMATION_SCHEMA.COLUMNS c - WHERE - c.TABLE_NAME = '#{view_tblnm}' - AND c.COLUMN_NAME = '#{views_real_column_name(table_name, ci[:name])}' - }.squish, "SCHEMA" + view_column = views_real_column_name(table_name, ci[:name]).downcase + default = default_functions[view_column] if view_column.present? end case default when nil @@ -582,17 +624,19 @@ def view_information(table_name) identifier = SQLServer::Utils.extract_identifiers(table_name) information_query_table = identifier.database.present? ? "[#{identifier.database}].[INFORMATION_SCHEMA].[VIEWS]" : "[INFORMATION_SCHEMA].[VIEWS]" view_info = select_one "SELECT * FROM #{information_query_table} WITH (NOLOCK) WHERE TABLE_NAME = #{quote(identifier.object)}", "SCHEMA" + if view_info view_info = view_info.with_indifferent_access if view_info[:VIEW_DEFINITION].blank? || view_info[:VIEW_DEFINITION].length == 4000 view_info[:VIEW_DEFINITION] = begin - select_values("EXEC sp_helptext #{identifier.object_quoted}", "SCHEMA").join + select_values("EXEC sp_helptext #{identifier.object_quoted}", "SCHEMA").join rescue warn "No view definition found, possible permissions problem.\nPlease run GRANT VIEW DEFINITION TO your_user;" nil - end + end end end + view_info end end @@ -601,7 +645,8 @@ def views_real_column_name(table_name, column_name) view_definition = view_information(table_name)[:VIEW_DEFINITION] return column_name unless view_definition - match_data = view_definition.match(/([\w-]*)\s+as\s+#{column_name}/im) + # Remove "CREATE VIEW ... AS SELECT ..." and then match the column name. + match_data = view_definition.sub(/CREATE\s+VIEW.*AS\s+SELECT\s/, '').match(/([\w-]*)\s+AS\s+#{column_name}\W/im) match_data ? match_data[1] : column_name end diff --git a/lib/active_record/connection_adapters/sqlserver_adapter.rb b/lib/active_record/connection_adapters/sqlserver_adapter.rb index a8cb14337..b35ecffbe 100644 --- a/lib/active_record/connection_adapters/sqlserver_adapter.rb +++ b/lib/active_record/connection_adapters/sqlserver_adapter.rb @@ -228,6 +228,10 @@ def supports_datetime_with_precision? true end + def supports_check_constraints? + true + end + def supports_json? @version_year >= 2016 end diff --git a/test/cases/coerced_tests.rb b/test/cases/coerced_tests.rb index e7996f968..b60aa57e7 100644 --- a/test/cases/coerced_tests.rb +++ b/test/cases/coerced_tests.rb @@ -1449,6 +1449,13 @@ def test_schema_dump_includes_decimal_options_coerced output = dump_all_table_schema([/^[^n]/]) assert_match %r{precision: 3,[[:space:]]+scale: 2,[[:space:]]+default: 2\.78}, output end + + # SQL Server formats the check constraint expression differently. + coerce_tests! :test_schema_dumps_check_constraints + def test_schema_dumps_check_constraints_coerced + constraint_definition = dump_table_schema("products").split(/\n/).grep(/t.check_constraint.*products_price_check/).first.strip + assert_equal 't.check_constraint "[price]>[discounted_price]", name: "products_price_check"', constraint_definition + end end class SchemaDumperDefaultsTest < ActiveRecord::TestCase @@ -2145,6 +2152,17 @@ def test_in_order_of_with_enums_keys_coerced Book.where(author_id: nil, name: nil).delete_all Book.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.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.connection.add_index(:books, [:author_id, :name], unique: true) + end end require "models/dashboard" @@ -2283,3 +2301,51 @@ class ActiveRecord::Encryption::EncryptableRecordTest < ActiveRecord::Encryption assert_not author.valid? end end + +module ActiveRecord + class Migration + class CheckConstraintTest < ActiveRecord::TestCase + # SQL Server formats the check constraint expression differently. + coerce_tests! :test_check_constraints + def test_check_constraints_coerced + check_constraints = @connection.check_constraints("products") + assert_equal 1, check_constraints.size + + constraint = check_constraints.first + assert_equal "products", constraint.table_name + assert_equal "products_price_check", constraint.name + assert_equal "[price]>[discounted_price]", constraint.expression + end + + # SQL Server formats the check constraint expression differently. + coerce_tests! :test_add_check_constraint + def test_add_check_constraint_coerced + @connection.add_check_constraint :trades, "quantity > 0" + + check_constraints = @connection.check_constraints("trades") + assert_equal 1, check_constraints.size + + constraint = check_constraints.first + assert_equal "trades", constraint.table_name + assert_equal "chk_rails_2189e9f96c", constraint.name + assert_equal "[quantity]>(0)", constraint.expression + end + + # SQL Server formats the check constraint expression differently. + coerce_tests! :test_remove_check_constraint + def test_remove_check_constraint_coerced + @connection.add_check_constraint :trades, "price > 0", name: "price_check" + @connection.add_check_constraint :trades, "quantity > 0", name: "quantity_check" + + assert_equal 2, @connection.check_constraints("trades").size + @connection.remove_check_constraint :trades, name: "quantity_check" + assert_equal 1, @connection.check_constraints("trades").size + + constraint = @connection.check_constraints("trades").first + assert_equal "trades", constraint.table_name + assert_equal "price_check", constraint.name + assert_equal "[price]>(0)", constraint.expression + end + end + end +end diff --git a/test/cases/migration_test_sqlserver.rb b/test/cases/migration_test_sqlserver.rb index 132d69115..74b14eb18 100644 --- a/test/cases/migration_test_sqlserver.rb +++ b/test/cases/migration_test_sqlserver.rb @@ -115,4 +115,22 @@ class MigrationTestSQLServer < ActiveRecord::TestCase refute_includes schemas, { "name" => "some schema" } end end + + describe 'creating stored procedure' do + it 'stored procedure contains inserts are created successfully' do + sql = <<-SQL + CREATE OR ALTER PROCEDURE do_some_task + AS + IF NOT EXISTS(SELECT * FROM sys.objects WHERE type = 'U' AND name = 'SomeTableName') + BEGIN + CREATE TABLE SomeTableName (SomeNum int PRIMARY KEY CLUSTERED); + INSERT INTO SomeTableName(SomeNum) VALUES(1); + END + SQL + + assert_nothing_raised { connection.execute(sql) } + ensure + connection.execute("DROP PROCEDURE IF EXISTS dbo.do_some_task;") + end + end end diff --git a/test/cases/view_test_sqlserver.rb b/test/cases/view_test_sqlserver.rb new file mode 100644 index 000000000..b371bd0b1 --- /dev/null +++ b/test/cases/view_test_sqlserver.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require "cases/helper_sqlserver" + +class ViewTestSQLServer < ActiveRecord::TestCase + let(:connection) { ActiveRecord::Base.connection } + + describe 'view with default values' do + before do + connection.drop_table :view_casing_table rescue nil + connection.create_table :view_casing_table, force: true do |t| + t.boolean :Default_Falsey, null: false, default: false + t.boolean :Default_Truthy, null: false, default: true + t.string :default_string_null, null: true, default: nil + t.string :default_string, null: false, default: "abc" + end + + connection.execute("DROP VIEW IF EXISTS view_casing_table_view;") + connection.execute <<-SQL + CREATE VIEW view_casing_table_view AS + SELECT id AS id, + default_falsey AS falsey, + default_truthy AS truthy, + default_string_null AS s_null, + default_string AS s + FROM view_casing_table + SQL + end + + it "default values are correct when column casing used in tables and views are different" do + klass = Class.new(ActiveRecord::Base) do + self.table_name = "view_casing_table_view" + end + + obj = klass.new + assert_equal false, obj.falsey + assert_equal true, obj.truthy + assert_equal "abc", obj.s + assert_nil obj.s_null + assert_equal 0, klass.count + + obj.save! + assert_equal false, obj.falsey + assert_equal true, obj.truthy + assert_equal "abc", obj.s + assert_nil obj.s_null + assert_equal 1, klass.count + end + end +end