From 8b1c0055cb68992c419837abe96fd8adfedfab95 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 15 Aug 2024 14:19:38 +0100 Subject: [PATCH] Fixes before abstract adapter refactor (#1214) --- Gemfile | 2 + .../sqlserver/core_ext/calculations.rb | 4 +- .../sqlserver/core_ext/finder_methods.rb | 2 +- .../sqlserver/schema_statements.rb | 184 ++++++++++-------- test/cases/coerced_tests.rb | 10 +- 5 files changed, 113 insertions(+), 89 deletions(-) diff --git a/Gemfile b/Gemfile index 0f435985e..341d6786b 100644 --- a/Gemfile +++ b/Gemfile @@ -18,6 +18,8 @@ if ENV["RAILS_SOURCE"] gemspec path: ENV["RAILS_SOURCE"] elsif ENV["RAILS_BRANCH"] gem "rails", github: "rails/rails", branch: ENV["RAILS_BRANCH"] +elsif ENV["RAILS_COMMIT"] + gem "rails", github: "rails/rails", ref: ENV["RAILS_COMMIT"] else # Need to get rails source because the gem doesn't include tests version = ENV["RAILS_VERSION"] || begin 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 8357fdbe4..5fafea101 100644 --- a/lib/active_record/connection_adapters/sqlserver/core_ext/calculations.rb +++ b/lib/active_record/connection_adapters/sqlserver/core_ext/calculations.rb @@ -10,9 +10,9 @@ module CoreExt module Calculations private - + def build_count_subquery(relation, column_name, distinct) - klass.with_connection do |connection| + model.with_connection do |connection| relation = relation.unscope(:order) if connection.sqlserver? super(relation, column_name, distinct) end diff --git a/lib/active_record/connection_adapters/sqlserver/core_ext/finder_methods.rb b/lib/active_record/connection_adapters/sqlserver/core_ext/finder_methods.rb index 6016369c2..db2d06708 100644 --- a/lib/active_record/connection_adapters/sqlserver/core_ext/finder_methods.rb +++ b/lib/active_record/connection_adapters/sqlserver/core_ext/finder_methods.rb @@ -11,7 +11,7 @@ module FinderMethods private def construct_relation_for_exists(conditions) - klass.with_connection do |connection| + model.with_connection do |connection| if connection.sqlserver? _construct_relation_for_exists(conditions) else diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index f85806478..a82a73df5 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -37,18 +37,16 @@ def indexes(table_name) data = select("EXEC sp_helpindex #{quote(table_name)}", "SCHEMA") rescue [] data.reduce([]) do |indexes, index| - index = index.with_indifferent_access - - if index[:index_description].match?(/primary key/) + if index['index_description'].match?(/primary key/) indexes else - name = index[:index_name] - unique = index[:index_description].match?(/unique/) + name = index['index_name'] + unique = index['index_description'].match?(/unique/) where = select_value("SELECT [filter_definition] FROM sys.indexes WHERE name = #{quote(name)}", "SCHEMA") orders = {} columns = [] - index[:index_keys].split(",").each do |column| + index['index_keys'].split(",").each do |column| column.strip! if column.end_with?("(-)") @@ -480,16 +478,15 @@ def initialize_native_database_types end def column_definitions(table_name) - identifier = database_prefix_identifier(table_name) - database = identifier.fully_qualified_database_quoted - view_exists = view_exists?(table_name) - view_tblnm = view_table_name(table_name) if view_exists + identifier = database_prefix_identifier(table_name) + database = identifier.fully_qualified_database_quoted + view_exists = view_exists?(table_name) if view_exists sql = <<~SQL 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)} + WHERE c.TABLE_NAME = #{quote(view_table_name(table_name))} SQL results = internal_exec_query(sql, "SCHEMA") default_functions = results.each.with_object({}) { |row, out| out[row["name"]] = row["default"] }.compact @@ -498,71 +495,93 @@ def column_definitions(table_name) sql = column_definitions_sql(database, identifier) binds = [] - nv128 = SQLServer::Type::UnicodeVarchar.new limit: 128 + nv128 = SQLServer::Type::UnicodeVarchar.new(limit: 128) binds << Relation::QueryAttribute.new("TABLE_NAME", identifier.object, nv128) binds << Relation::QueryAttribute.new("TABLE_SCHEMA", identifier.schema, nv128) unless identifier.schema.blank? + results = internal_exec_query(sql, "SCHEMA", binds) + raise ActiveRecord::StatementInvalid, "Table '#{table_name}' doesn't exist" if results.empty? columns = results.map do |ci| - ci = ci.symbolize_keys - ci[:_type] = ci[:type] - ci[:table_name] = view_tblnm || table_name - ci[:type] = case ci[:type] - when /^bit|image|text|ntext|datetime$/ - ci[:type] - when /^datetime2|datetimeoffset$/i - "#{ci[:type]}(#{ci[:datetime_precision]})" - when /^time$/i - "#{ci[:type]}(#{ci[:datetime_precision]})" - when /^numeric|decimal$/i - "#{ci[:type]}(#{ci[:numeric_precision]},#{ci[:numeric_scale]})" - when /^float|real$/i - "#{ci[:type]}" - when /^char|nchar|varchar|nvarchar|binary|varbinary|bigint|int|smallint$/ - ci[:length].to_i == -1 ? "#{ci[:type]}(max)" : "#{ci[:type]}(#{ci[:length]})" - else - ci[:type] - end - ci[:default_value], - ci[:default_function] = begin - default = ci[:default_value] - if default.nil? && view_exists - 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 - [nil, nil] - when /\A\((\w+\(\))\)\Z/ - default_function = Regexp.last_match[1] - [nil, default_function] - when /\A\(N'(.*)'\)\Z/m - string_literal = SQLServer::Utils.unquote_string(Regexp.last_match[1]) - [string_literal, nil] - when /CREATE DEFAULT/mi - [nil, nil] - else - type = case ci[:type] - when /smallint|int|bigint/ then ci[:_type] - else ci[:type] - end - value = default.match(/\A\((.*)\)\Z/m)[1] - value = select_value("SELECT CAST(#{value} AS #{type}) AS value", "SCHEMA") - [value, nil] - end + col = { + name: ci["name"], + numeric_scale: ci["numeric_scale"], + numeric_precision: ci["numeric_precision"], + datetime_precision: ci["datetime_precision"], + collation: ci["collation"], + ordinal_position: ci["ordinal_position"], + length: ci["length"] + } + + col[:table_name] = view_table_name(table_name) || table_name + col[:type] = column_type(ci: ci) + col[:default_value], col[:default_function] = default_value_and_function(default: ci['default_value'], + name: ci['name'], + type: col[:type], + original_type: ci['type'], + view_exists: view_exists, + table_name: table_name, + default_functions: default_functions) + + col[:null] = ci['is_nullable'].to_i == 1 + col[:is_primary] = ci['is_primary'].to_i == 1 + + if [true, false].include?(ci['is_identity']) + col[:is_identity] = ci['is_identity'] + else + col[:is_identity] = ci['is_identity'].to_i == 1 end - ci[:null] = ci[:is_nullable].to_i == 1 - ci.delete(:is_nullable) - ci[:is_primary] = ci[:is_primary].to_i == 1 - ci[:is_identity] = ci[:is_identity].to_i == 1 unless [TrueClass, FalseClass].include?(ci[:is_identity].class) - ci + + col end - # Since Rails 7, it's expected that all adapter raise error when table doesn't exists. - # I'm not aware of the possibility of tables without columns on SQL Server (postgres have those). - # Raise error if the method return an empty array - columns.tap do |result| - raise ActiveRecord::StatementInvalid, "Table '#{table_name}' doesn't exist" if result.empty? + columns + end + + def default_value_and_function(default:, name:, type:, original_type:, view_exists:, table_name:, default_functions:) + if default.nil? && view_exists + view_column = views_real_column_name(table_name, name).downcase + default = default_functions[view_column] if view_column.present? + end + + case default + when nil + [nil, nil] + when /\A\((\w+\(\))\)\Z/ + default_function = Regexp.last_match[1] + [nil, default_function] + when /\A\(N'(.*)'\)\Z/m + string_literal = SQLServer::Utils.unquote_string(Regexp.last_match[1]) + [string_literal, nil] + when /CREATE DEFAULT/mi + [nil, nil] + else + type = case type + when /smallint|int|bigint/ then original_type + else type + end + value = default.match(/\A\((.*)\)\Z/m)[1] + value = select_value("SELECT CAST(#{value} AS #{type}) AS value", "SCHEMA") + [value, nil] + end + end + + def column_type(ci:) + case ci['type'] + when /^bit|image|text|ntext|datetime$/ + ci['type'] + when /^datetime2|datetimeoffset$/i + "#{ci['type']}(#{ci['datetime_precision']})" + when /^time$/i + "#{ci['type']}(#{ci['datetime_precision']})" + when /^numeric|decimal$/i + "#{ci['type']}(#{ci['numeric_precision']},#{ci['numeric_scale']})" + when /^float|real$/i + "#{ci['type']}" + when /^char|nchar|varchar|nvarchar|binary|varbinary|bigint|int|smallint$/ + ci['length'].to_i == -1 ? "#{ci['type']}(max)" : "#{ci['type']}(#{ci['length']})" + else + ci['type'] end end @@ -701,25 +720,26 @@ def lowercase_schema_reflection_sql(node) def view_table_name(table_name) view_info = view_information(table_name) - view_info ? get_table_name(view_info["VIEW_DEFINITION"]) : table_name + view_info.present? ? get_table_name(view_info["VIEW_DEFINITION"]) : table_name end def view_information(table_name) @view_information ||= {} + @view_information[table_name] ||= begin 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 - rescue - warn "No view definition found, possible permissions problem.\nPlease run GRANT VIEW DEFINITION TO your_user;" - nil - end + + view_info = select_one("SELECT * FROM #{information_query_table} WITH (NOLOCK) WHERE TABLE_NAME = #{quote(identifier.object)}", "SCHEMA").to_h + + if view_info.present? + 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 + rescue + warn "No view definition found, possible permissions problem.\nPlease run GRANT VIEW DEFINITION TO your_user;" + nil + end end end @@ -728,8 +748,8 @@ def view_information(table_name) end def views_real_column_name(table_name, column_name) - view_definition = view_information(table_name)[:VIEW_DEFINITION] - return column_name unless view_definition + view_definition = view_information(table_name)['VIEW_DEFINITION'] + return column_name if view_definition.blank? # 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) diff --git a/test/cases/coerced_tests.rb b/test/cases/coerced_tests.rb index 21d419c9b..44c7d2211 100644 --- a/test/cases/coerced_tests.rb +++ b/test/cases/coerced_tests.rb @@ -2419,7 +2419,9 @@ class QueryLogsTest < ActiveRecord::TestCase # SQL requires double single-quotes. coerce_tests! :test_sql_commenter_format def test_sql_commenter_format_coerced - ActiveRecord::QueryLogs.update_formatter(:sqlcommenter) + ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter + ActiveRecord::QueryLogs.tags = [:application] + assert_queries_match(%r{/\*application=''active_record''\*/}) do Dashboard.first end @@ -2428,7 +2430,7 @@ def test_sql_commenter_format_coerced # SQL requires double single-quotes. coerce_tests! :test_sqlcommenter_format_value def test_sqlcommenter_format_value_coerced - ActiveRecord::QueryLogs.update_formatter(:sqlcommenter) + ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter ActiveRecord::QueryLogs.tags = [ :application, @@ -2443,7 +2445,7 @@ def test_sqlcommenter_format_value_coerced # SQL requires double single-quotes. coerce_tests! :test_sqlcommenter_format_value_string_coercible def test_sqlcommenter_format_value_string_coercible_coerced - ActiveRecord::QueryLogs.update_formatter(:sqlcommenter) + ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter ActiveRecord::QueryLogs.tags = [ :application, @@ -2458,7 +2460,7 @@ def test_sqlcommenter_format_value_string_coercible_coerced # SQL requires double single-quotes. coerce_tests! :test_sqlcommenter_format_allows_string_keys def test_sqlcommenter_format_allows_string_keys_coerced - ActiveRecord::QueryLogs.update_formatter(:sqlcommenter) + ActiveRecord::QueryLogs.tags_formatter = :sqlcommenter ActiveRecord::QueryLogs.tags = [ :application,