Skip to content

Conversation

@metaskills
Copy link
Member

I made a mistake and made _type_cast do quoting too. This is now fixed and we have greatly simplified it along with _quote and we now use these two to cast and quote as needed in our sp_executesql helpers.

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 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.

cc @sgrif

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.
@metaskills
Copy link
Member Author

After this work, I can finally start to attach the ~100 errors in the full ActiveRecord test suite knowing that the type foundation is stronger for us. @sgrif, would love to get your thoughts on that query attribute uniqueness cast value mentioned above at some point.

@metaskills
Copy link
Member Author

Here is an example of a test and the bind attr I get for uniqueness validations. Ideally the cast_type would have been used here vs a basic value so we can understand the true type and do what we need for cast/quoting in sq_executesql.

Topic.validates_uniqueness_of(:title)
topic = Topic.new title: 'abc'
assert topic.valid?

=> #<ActiveRecord::Relation::QueryAttribute:0x007fa11bc34128
 @name="title",
 @original_attribute=nil,
 @type=#<ActiveModel::Type::Value:0x007fa11bc34150 @limit=nil, @precision=nil, @scale=nil>,
 @value="N'abc'",
 @value_before_type_cast="N'abc'",
 @value_for_database="N'abc'">

@metaskills metaskills merged commit 096e1b7 into master Dec 28, 2016
@metaskills metaskills deleted the GenericTypeCast branch December 28, 2016 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants