Open PaulCharlton opened 4 years ago
Does :use_utf16
already work like you want? https://github.com/rails-sqlserver/tiny_tds/blob/master/lib/tiny_tds/client.rb#L52
all settings in DATABASE_URL, such as "?azure=false" should return "expected" config result of boolean false, consistent with ActiveRecord conversion from database native string to Ruby boolean.
Where is the code responsible for this?
I already started testing a WIP PR for this ... new unit tests first.
:use_utf16
is closer in behavior, but still not consistent with rails handling of underlying database fields, and the use_utf16 defaults to true
when the config is nil
I was thinking code something like the following (half baked) psuedocode:
FALSE_VALUES = [
false, 0,
"", :"",
"0", :0,
"f", :f,
"F", :F,
"false", :false,
"FALSE", :FALSE,
"off", :off,
"OFF", :OFF,
].to_set.freeze
...
def initialize(opts = {})
...
opts[:azure] = to_boolean(:azure)
opts[:contained] = to_boolean(:contained)
opts[:use_utf16] = opts[:use_utf16].nil? || to_boolean(:use_utf16)
...
end
...
def to_boolean(opt)
// something for handling nil
return !FALSE_VALUES.include?(opts[:opt].to_s)
end
the corresponding rails typecasting code is in activemodel/lib/active_model/type/boolean.rb
@ github where "truthy" is semi-defined by the behavior therein.
I'm somewhat skeptical that that code should live here in the tiny_tds project. To a reasonable degree, this project should be web framework agnostic and follow normal gem paradigms. Treating "F"
as false
doesn't seem like a normal gem thing, it seems like some kind of Rails configuration idea.
Do other database gems do this kind of thing? Should that code live in the activerecord sql server adapter?
wrote up an issue for rails/rails
to address the ability to unset
or remove a config key via the contents of DATABASE_URL here -> https://github.com/rails/rails/issues/38748
@aharpervc good point
see: https://github.com/rails/rails/issues/9097#issuecomment-14023312 and https://github.com/rails/rails/commit/e54acf1308e2e4df047bf90798208e03e1370098
which explicitly defer type-coercion into the adapter layer -- however, the config option needs to be a config option key that the adapter layer is aware of, and TinyTDS has not explicitly published these newer settings into the sqlserver adapter layer as yet. (actually, from reading the code, and later comments, the type-coercion is deferred to the point of use
... and in our current example, TinyTDS is the first point of use
unless we want to train the SQLServer adapter how to recognize those new config keys
unless we want to train the SQLServer adapter how to recognize those new config keys
Yeah, I'm thinking that is possible the preferable way to do it. That seems like the piece of the stack that could reasonably be responsible for handling "rails" <-> "database driver" connectivity, which in this case means mapping various properties and their data types
@aharpervc after looking through the code in the activerecord-sqlserver-adapter, I concur that it should be responsible for the type-casting as it is already reading (aka first use
) those values from the config hash and passing them to the initialization of the TinyTDS client. (with the exception of :use_rtf16, which it does not interpret, and I am seeing how a rails app can ever pass a meaningful value of :use_rtf16 into TinyTDS via activerecord-sqlserver-adapter
another thought -- usually when I have two (or more) pieces of code that need a shared understanding of data format, I write a schema and then either a code generator from the schema to each user of that data, or write a schema interpreter in each user, and share the schema amongst them. In this particular case, we have at least 3 pieces of code that need a shared understanding of the config attributes --- (1) TinyTDS, (2) AR-SqlServer-Adapter, (3) application code (database.yml), (4) Devops config (env URL).
TinyTDS v2.1.2 and current HEAD
Current State:
At the moment, the only "false" values for these configs are "nil" and the boolean value
false
, whereas all non-nil string values, including the empty string and the string "false" are interpreted as the boolean valuetrue
.This presents several issues:
database.yml
2.1 setting any of the aforementioned options indatabase.yml
to any value other than nil can not be converted to a booleanfalse
value by anything set in 'DATABASE_URL', even if 'DATABASE_URL' contains "?azure=false", it will be interpreted by TinyTDS as booleantrue
Rails Reference and also Discussion here
Desired State:
false
, consistent with ActiveRecord conversion from database native string to Ruby boolean.