search

rails-sqlserver / tiny_tds

TinyTDS - Simple and fast FreeTDS bindings for Ruby using DB-Library.
Other
607 stars 189 forks source link

Proposal: Allow Setting @query_options per Client #120

Open wbond opened 11 years ago

wbond commented 11 years ago

I had just upgraded a rails app using tiny_tds and activerecord-sqlserver-adapter and ran into a bug cause by me calling the following in my config/application.rb:

TinyTds::Client.default_query_options[:symbolize_keys] = true

I do this because I have some of my own TinyTds::Client instances and I don't want to pass :symbolize_keys => true to each execute call.

It seems that with Rails 3.2.13 something has changed that presents issues:

/home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/result.rb:33:in `dup': can't dup Symbol (TypeError)
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/result.rb:33:in `block in hash_rows'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/result.rb:33:in `map'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/result.rb:33:in `hash_rows'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/result.rb:20:in `each'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-sqlserver-adapter-3.2.9/lib/active_record/connection_adapters/sqlserver/database_statements.rb:297:in `to_a'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-sqlserver-adapter-3.2.9/lib/active_record/connection_adapters/sqlserver/database_statements.rb:297:in `select'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/database_statements.rb:18:in `select_all'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/query_cache.rb:63:in `select_all'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/database_statements.rb:24:in `select_one'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/database_statements.rb:30:in `select_value'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-sqlserver-adapter-3.2.9/lib/active_record/connection_adapters/sqlserver_adapter.rb:205:in `initialize'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-sqlserver-adapter-3.2.9/lib/active_record/connection_adapters/sqlserver_adapter.rb:40:in `new'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-sqlserver-adapter-3.2.9/lib/active_record/connection_adapters/sqlserver_adapter.rb:40:in `sqlserver_connection'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_pool.rb:315:in `new_connection'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_pool.rb:325:in `checkout_new_connection'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_pool.rb:247:in `block (2 levels) in checkout'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_pool.rb:242:in `loop'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_pool.rb:242:in `block in checkout'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/1.9.1/monitor.rb:211:in `mon_synchronize'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_pool.rb:239:in `checkout'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_pool.rb:102:in `block in connection'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/1.9.1/monitor.rb:211:in `mon_synchronize'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_pool.rb:101:in `connection'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_pool.rb:410:in `retrieve_connection'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_specification.rb:171:in `retrieve_connection'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_specification.rb:145:in `connection'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activerecord-3.2.13/lib/active_record/railtie.rb:88:in `block in <class:Railtie>'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/initializable.rb:30:in `instance_exec'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/initializable.rb:30:in `run'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/initializable.rb:55:in `block in run_initializers'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/initializable.rb:54:in `each'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/initializable.rb:54:in `run_initializers'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/application.rb:136:in `initialize!'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/railtie/configurable.rb:30:in `method_missing'
    from /home/wbond/storage/git/forklift/config/environment.rb:5:in `<top (required)>'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activesupport-3.2.13/lib/active_support/dependencies.rb:251:in `require'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activesupport-3.2.13/lib/active_support/dependencies.rb:251:in `block in require'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activesupport-3.2.13/lib/active_support/dependencies.rb:236:in `load_dependency'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/activesupport-3.2.13/lib/active_support/dependencies.rb:251:in `require'
    from /home/wbond/storage/git/forklift/config.ru:3:in `block in <main>'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/rack-1.4.5/lib/rack/builder.rb:51:in `instance_eval'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/rack-1.4.5/lib/rack/builder.rb:51:in `initialize'
    from /home/wbond/storage/git/forklift/config.ru:in `new'
    from /home/wbond/storage/git/forklift/config.ru:in `<main>'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/rack-1.4.5/lib/rack/builder.rb:40:in `eval'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/rack-1.4.5/lib/rack/builder.rb:40:in `parse_file'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/rack-1.4.5/lib/rack/server.rb:200:in `app'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/commands/server.rb:46:in `app'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/rack-1.4.5/lib/rack/server.rb:304:in `wrapped_app'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/rack-1.4.5/lib/rack/server.rb:254:in `start'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/commands/server.rb:70:in `start'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/commands.rb:55:in `block in <top (required)>'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/commands.rb:50:in `tap'
    from /home/wbond/.rbenv/versions/1.9.3-p194/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/commands.rb:50:in `<top (required)>'
    from script/rails:6:in `require'
    from script/rails:6:in `<main>'

This was resolved by removing the default_query_options call in my application.rb.

Now, it seems that TinyTds::Client copies default_query_options into @query_options when being initialized.

What about allowing users to pass :query_options as a param to TinyTds::Client.new and have that merge over the default_query_options?

If this, or something like this sounds like a good idea, I'd be happy to submit a pull request.

wbond commented 11 years ago

@metaskills Thoughts?

metaskills commented 11 years ago

Sounds OK to me.