Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't barf on the datetime2 data type when connecting to MS SQL 2008 #233

Closed
wants to merge 1 commit into from

Conversation

trak3r
Copy link

@trak3r trak3r commented Sep 12, 2012

Don't barf on the datetime2 data type when connecting to MS SQL 2008

@bwalsh
Copy link

bwalsh commented Dec 4, 2012

Running into this problem. Is there anything blocking this pull? Anything I can do to help?

@gregors
Copy link
Contributor

gregors commented Dec 28, 2012

I'm also running into this issue. Also this is a duplicate of #116 - are the build failures related to this pull request? What do we need to do to get this moving? A failing test for 2008?

@gregors
Copy link
Contributor

gregors commented Jan 8, 2013

I guess ms sql doesn't get any love around here?

@kares
Copy link
Member

kares commented Jan 17, 2013

taking quite a while, I'll try to manage but it's hard without a MS-SQL DB ... a (specific) tests would be great to make sure it ain't breaking anything else ... could someone add one please (I'm willing to get this up than - assuming no test failures) ?
@gregors also it gets all the love we can give it :) ... unfortunately it's hard to install SQLServer on a "free" OS :(

@gregors
Copy link
Contributor

gregors commented Jan 17, 2013

@kares - I meant no disrespect. I really appreciate all the hard work you guys are doing!!! I'm more than willing to test MS-SQL DB. The project is making my own life much more enjoyable so I'll be glad to do whatever I can. I met with @nicksieger briefly at Rubyconf in Denver about some ideas I had regarding the MS SQL portion of the code. I'd definitely like to be more involved in that area.

@gregors
Copy link
Contributor

gregors commented Jan 17, 2013

@kares I'll work on getting a failing test for this issue

@kares
Copy link
Member

kares commented Feb 21, 2013

so I do have a MS-SQL but failed to reproduce this so far ... if someone could provide a failing test-case pls otherwise I do not see any reason to merge this since the regex applies for all adapters (known and unknown) not only MS-SQL

@jhiggins
Copy link

This is certainly causing problems in my rails application.

I spent a bit of time this weekend trying to create a failing test, but I wasn't able to replicate it in a unit test. I don't have the code available at the moment.

@gregors, what were you seeing?

@gregors
Copy link
Contributor

gregors commented Feb 27, 2013

@jhiggins I'm no longer seeing this issue with versino 1.2.7 - though I'm seeing an issue with Model.first (which has been fixed in issue #329 in master)

@gregors
Copy link
Contributor

gregors commented Feb 27, 2013

OK, yes the problem looks like it still exists - but it's more difficult to see. When creating a table from within rails using a migration the type datetime is created - when using activerecord against an existing database with uses the newer datetime2 type it most definitely fails - for instance I was using activerecord to pump test data into an asp.net MVC application (which suses datetime2) via factorty_girl, boom fail all day long.

So I guess a valid test is to manually create a table with datetime2 and watch it blow up?

@gregors
Copy link
Contributor

gregors commented Mar 1, 2013

I created another test in a pull request, I am not seeing any issues on master

@kares
Copy link
Member

kares commented Mar 4, 2013

As #116 was not reproduced probably due some changes on master that fixed the issue, I'm closing this one.
Thank you for your attempt to fix the issue but it felt like it needed some hand tuning exclusive to MS-SQL adapter.
Please if someone runs into the issue - provide us with a failing test case / piece of code so we can reproduce ...

@jhiggins
Copy link

jhiggins commented Mar 6, 2013

Still failing in my rail application with 1.2.7, will see if I can make a standalone test.

Your bundle is complete! Use bundle show [gemname] to see where a bundled gem is installed.

jhiggins-imac-3:nanny-office jhiggins$ rails server
=> Booting WEBrick
=> Rails 3.2.11 application starting in development on http://0.0.0.0:3000
=> Call with -d to detach
=> Ctrl-C to shutdown server
Exiting
RuntimeError: unable to choose type for timestamp from:
["datetime2", "datetime"]
                                       choose_type at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.2.7/lib/arjdbc/jdbc/type_converter.rb:116
                                 choose_best_types at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.2.7/lib/arjdbc/jdbc/type_converter.rb:91
                                              each at org/jruby/RubyArray.java:1612
                                 choose_best_types at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.2.7/lib/arjdbc/jdbc/type_converter.rb:90
                         set_native_database_types at arjdbc/jdbc/RubyJdbcConnection.java:539
                                        initialize at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.2.7/lib/arjdbc/jdbc/connection.rb:84
                                        initialize at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.2.7/lib/arjdbc/jdbc/adapter.rb:31
                                   jdbc_connection at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.2.7/lib/arjdbc/jdbc/connection_methods.rb:6
                                  mssql_connection at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.2.7/lib/arjdbc/mssql/connection_methods.rb:34
                                          __send__ at org/jruby/RubyBasicObject.java:1673
                                              send at org/jruby/RubyKernel.java:2081
                                    new_connection at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-3.2.11/lib/active_record/connection_adapters/abstract/connection_pool.rb:315
                           checkout_new_connection at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-3.2.11/lib/active_record/connection_adapters/abstract/connection_pool.rb:325
                                          checkout at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-3.2.11/lib/active_record/connection_adapters/abstract/connection_pool.rb:247
                                              loop at org/jruby/RubyKernel.java:1390
                                          checkout at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-3.2.11/lib/active_record/connection_adapters/abstract/connection_pool.rb:242
                                   mon_synchronize at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/1.9/monitor.rb:211
                                          checkout at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-3.2.11/lib/active_record/connection_adapters/abstract/connection_pool.rb:239
                                        connection at /Users/jhiggins/.rbenv/versions/jruby-1.7.0-rc2/lib/ruby/gems/shared/gems/activerecord-3.2.11/lib/active_record/connection_adapters/abstract/connection_pool.rb:102

@kares kares mentioned this pull request Apr 28, 2013
@slorek
Copy link
Contributor

slorek commented May 8, 2013

Just to confirm that this issue still exists in master, when connecting to Azure SQL Database. The fix in the pull request works for me. Please consider merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants