From c0bb1761b48e405f4bdb0618bb1217070ad5be78 Mon Sep 17 00:00:00 2001
From: Ken Collins <ken@metaskills.net>
Date: Tue, 27 Dec 2016 21:23:00 -0500
Subject: [PATCH] Refactor `_type_cast` & `_quote`.

I made a mistake and made `_type_cast` do quoting too. This is now fixed and we have greatly simplified it and `_quote` and use these two to cast and quote as needed in our sp_executesql helpers since we need to format both cast and quoted SQL strings.

Changed `sp_executesql_sql_type` to have a default `String` case to `nvarchar(max)` not ideal and there are places where we prefer the bind contain the proper SQLServer type. For example, uniquness validations yielding a `ActiveRecord::Relation::QueryAttribute` with a [basic cast type](https://github.com/rails/rails/blob/5-0-stable/activerecord/lib/active_record/validations/uniqueness.rb#L80) vs the in scope `cast_type` further up.

Added `sp_executesql_sql_param` which looks out for all `SQLServer::Data` and quotes it accordingly. For example, date & time objects need single quote '' vs. national N'' quoting.

* New base `SQLServer::Data` shared for all types. Delegates quoting decisoin back to type.
* Time types use variouse flavors of (Date|Time)#strptime in concert with SQL Server date/time formats in fast string processing.
---
 .../sqlserver/database_statements.rb          | 15 +++++++--
 .../connection_adapters/sqlserver/quoting.rb  | 13 ++------
 .../sqlserver/type/data.rb                    |  4 +++
 .../sqlserver/type/date.rb                    | 22 +++++++++----
 .../sqlserver/type/datetime.rb                | 32 +++++++++++--------
 .../sqlserver/type/datetimeoffset.rb          | 18 +----------
 .../sqlserver/type/smalldatetime.rb           |  4 +++
 .../sqlserver/type/time.rb                    | 16 ++++++----
 test/cases/column_test_sqlserver.rb           |  9 +++++-
 9 files changed, 78 insertions(+), 55 deletions(-)

diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb
index 9083ead03..aa6154b3d 100644
--- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb
+++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb
@@ -240,7 +240,7 @@ def sp_executesql_types_and_parameters(binds)
           types, params = [], []
           binds.each_with_index do |attr, index|
             types << "@#{index} #{sp_executesql_sql_type(attr)}"
-            params << type_cast(attr.value_for_database)
+            params << sp_executesql_sql_param(attr)
           end
           [types, params]
         end
@@ -250,9 +250,20 @@ def sp_executesql_sql_type(attr)
           case value = attr.value_for_database
           when Numeric
             'int'.freeze
+          when String
+            'nvarchar(max)'.freeze
           else
             raise TypeError, "sp_executesql_sql_type can not find sql type for attr #{attr.inspect}"
-            quote(value)
+          end
+        end
+
+        def sp_executesql_sql_param(attr)
+          case attr.value_for_database
+          when Type::Binary::Data,
+               ActiveRecord::Type::SQLServer::Data
+            quote(attr.value_for_database)
+          else
+            quote(type_cast(attr.value_for_database))
           end
         end
 
diff --git a/lib/active_record/connection_adapters/sqlserver/quoting.rb b/lib/active_record/connection_adapters/sqlserver/quoting.rb
index a3e6e83fb..95d26ffa8 100644
--- a/lib/active_record/connection_adapters/sqlserver/quoting.rb
+++ b/lib/active_record/connection_adapters/sqlserver/quoting.rb
@@ -86,17 +86,10 @@ def _quote(value)
 
         def _type_cast(value)
           case value
-          when nil
-            "NULL"
-          when Symbol
-            _quote(value.to_s)
-          when String, ActiveSupport::Multibyte::Chars
-            _quote(value)
-          when Type::Binary::Data
-            _quote(value)
           when ActiveRecord::Type::SQLServer::Data
-            _quote(value)
-          else super
+            value.to_s
+          else
+            super
           end
         end
 
diff --git a/lib/active_record/connection_adapters/sqlserver/type/data.rb b/lib/active_record/connection_adapters/sqlserver/type/data.rb
index 609df8960..a96d54cd1 100644
--- a/lib/active_record/connection_adapters/sqlserver/type/data.rb
+++ b/lib/active_record/connection_adapters/sqlserver/type/data.rb
@@ -19,6 +19,10 @@ def to_s
           end
           alias_method :to_str, :to_s
 
+          def inspect
+            @value.inspect
+          end
+
         end
       end
     end
diff --git a/lib/active_record/connection_adapters/sqlserver/type/date.rb b/lib/active_record/connection_adapters/sqlserver/type/date.rb
index 851fe2d0f..492377ef7 100644
--- a/lib/active_record/connection_adapters/sqlserver/type/date.rb
+++ b/lib/active_record/connection_adapters/sqlserver/type/date.rb
@@ -10,13 +10,12 @@ def sqlserver_type
 
           def serialize(value)
             return unless value.present?
-            return value if value.is_a?(Data)
-            Data.new super, self
+            date = value.to_s(:_sqlserver_dateformat)
+            Data.new date, self
           end
 
           def deserialize(value)
-            return value.value if value.is_a?(Data)
-            super
+            value.is_a?(Data) ? super(value.value) : super
           end
 
           def type_cast_for_schema(value)
@@ -24,8 +23,19 @@ def type_cast_for_schema(value)
           end
 
           def quoted(value)
-            date = value.to_s(:_sqlserver_dateformat)
-            Utils.quote_string_single(date)
+            Utils.quote_string_single(value)
+          end
+
+          private
+
+          def fast_string_to_date(string)
+            ::Date.strptime(string, fast_string_to_date_format)
+          rescue ArgumentError
+            super
+          end
+
+          def fast_string_to_date_format
+            ::Date::DATE_FORMATS[:_sqlserver_dateformat]
           end
 
         end
diff --git a/lib/active_record/connection_adapters/sqlserver/type/datetime.rb b/lib/active_record/connection_adapters/sqlserver/type/datetime.rb
index 3bf1a7b7d..480547022 100644
--- a/lib/active_record/connection_adapters/sqlserver/type/datetime.rb
+++ b/lib/active_record/connection_adapters/sqlserver/type/datetime.rb
@@ -12,13 +12,15 @@ def sqlserver_type
 
           def serialize(value)
             return super unless value.acts_like?(:time)
-            Data.new super, self
+            datetime = super.to_s(:_sqlserver_datetime).tap do |v|
+              fraction = quote_fractional(value)
+              v << ".#{fraction}" unless fraction.to_i.zero?
+            end
+            Data.new datetime, self
           end
 
           def deserialize(value)
-            datetime = value.is_a?(Data) ? value.value : super
-            return unless datetime
-            zone_conversion(datetime)
+            value.is_a?(Data) ? super(value.value) : super
           end
 
           def type_cast_for_schema(value)
@@ -26,12 +28,7 @@ def type_cast_for_schema(value)
           end
 
           def quoted(value)
-            datetime = value.to_s(:_sqlserver_datetime)
-            datetime = "#{datetime}".tap do |v|
-              fraction = quote_fractional(value)
-              v << ".#{fraction}" unless fraction.to_i.zero?
-            end
-            Utils.quote_string_single(datetime)
+            Utils.quote_string_single(value)
           end
 
           private
@@ -42,9 +39,18 @@ def cast_value(value)
             apply_seconds_precision(value)
           end
 
-          def zone_conversion(value)
-            method = ActiveRecord::Base.default_timezone == :utc ? :getutc : :getlocal
-            value.respond_to?(method) ? value.send(method) : value
+          def fast_string_to_time(string)
+            fast_string_to_time_zone.strptime(string, fast_string_to_time_format).time
+          rescue ArgumentError
+            super
+          end
+
+          def fast_string_to_time_format
+            "#{::Time::DATE_FORMATS[:_sqlserver_datetime]}.%N".freeze
+          end
+
+          def fast_string_to_time_zone
+            ::Time.zone || ActiveSupport::TimeZone.new('UTC')
           end
 
         end
diff --git a/lib/active_record/connection_adapters/sqlserver/type/datetimeoffset.rb b/lib/active_record/connection_adapters/sqlserver/type/datetimeoffset.rb
index d1b1a88ba..9564dadda 100644
--- a/lib/active_record/connection_adapters/sqlserver/type/datetimeoffset.rb
+++ b/lib/active_record/connection_adapters/sqlserver/type/datetimeoffset.rb
@@ -8,28 +8,12 @@ def type
             :datetimeoffset
           end
 
-          def type_cast_for_schema(value)
-            serialize(value).inspect
-          end
-
           def sqlserver_type
             "datetimeoffset(#{precision.to_i})"
           end
 
           def quoted(value)
-            datetime = value.to_s(:_sqlserver_datetimeoffset)
-            Utils.quote_string_single(datetime)
-          end
-
-          private
-
-          def fast_string_to_time(string)
-            dateformat = ::Time::DATE_FORMATS[:_sqlserver_dateformat]
-            ::Time.strptime string, "#{dateformat} %H:%M:%S.%N %:z"
-          end
-
-          def zone_conversion(value)
-            value
+            Utils.quote_string_single(value)
           end
 
         end
diff --git a/lib/active_record/connection_adapters/sqlserver/type/smalldatetime.rb b/lib/active_record/connection_adapters/sqlserver/type/smalldatetime.rb
index 67070e077..22c6c1856 100644
--- a/lib/active_record/connection_adapters/sqlserver/type/smalldatetime.rb
+++ b/lib/active_record/connection_adapters/sqlserver/type/smalldatetime.rb
@@ -14,6 +14,10 @@ def sqlserver_type
 
           private
 
+          def fast_string_to_time_format
+            ::Time::DATE_FORMATS[:_sqlserver_datetime]
+          end
+
           def apply_seconds_precision(value)
             value.change usec: 0
           end
diff --git a/lib/active_record/connection_adapters/sqlserver/type/time.rb b/lib/active_record/connection_adapters/sqlserver/type/time.rb
index 2a23fcb44..5d8ca3c0f 100644
--- a/lib/active_record/connection_adapters/sqlserver/type/time.rb
+++ b/lib/active_record/connection_adapters/sqlserver/type/time.rb
@@ -8,7 +8,15 @@ class Time < ActiveRecord::Type::Time
 
           def serialize(value)
             return super unless value.acts_like?(:time)
-            Data.new super, self
+            time = value.to_s(:_sqlserver_time).tap do |v|
+              fraction = quote_fractional(value)
+              v << ".#{fraction}" unless fraction.to_i.zero?
+            end
+            Data.new time, self
+          end
+
+          def deserialize(value)
+            value.is_a?(Data) ? super(value.value) : super
           end
 
           def type_cast_for_schema(value)
@@ -20,11 +28,7 @@ def sqlserver_type
           end
 
           def quoted(value)
-            time = value.to_s(:_sqlserver_time).tap do |v|
-              fraction = quote_fractional(value)
-              v << ".#{fraction}" unless fraction.to_i.zero?
-            end
-            Utils.quote_string_single(time)
+            Utils.quote_string_single(value)
           end
 
           private
diff --git a/test/cases/column_test_sqlserver.rb b/test/cases/column_test_sqlserver.rb
index f787eefb4..d3a2e217e 100644
--- a/test/cases/column_test_sqlserver.rb
+++ b/test/cases/column_test_sqlserver.rb
@@ -281,7 +281,14 @@ def assert_obj_set_and_save(attribute, value)
       type.limit.must_be_nil
       type.precision.must_be_nil
       type.scale.must_be_nil
-      # Can cast strings.
+      # Can cast strings. SQL Server format.
+      obj.date = '04-01-0001'
+      obj.date.must_equal               Date.civil(0001, 4, 1)
+      obj.save!
+      obj.date.must_equal               Date.civil(0001, 4, 1)
+      obj.reload
+      obj.date.must_equal               Date.civil(0001, 4, 1)
+      # Can cast strings. ISO format.
       obj.date = '0001-04-01'
       obj.date.must_equal               Date.civil(0001, 4, 1)
       obj.save!