-
Notifications
You must be signed in to change notification settings - Fork 564
Apply monkey patches for SQL Server connections only #933
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
Apply monkey patches for SQL Server connections only #933
Conversation
|
Hmm. Appveyor CI is failing with one failing test, but that same test fails for me locally on On |
|
Yeah this fix looks fine to me. The |
lib/active_record/connection_adapters/sqlserver/core_ext/attribute_methods.rb
Outdated
Show resolved
Hide resolved
|
Also, could you rebase your branch and add a changelog entry? |
53d91c6 to
3e9a38c
Compare
Recent versions of Rails support multiple database connections within the same app. It is possible for these connections to use different adapters. For example, one adapter may use SQL Server, and another uses PostgreSQL. This gem applies some monkey patches to ActiveRecord for SQL Server compatibility. These patches could break other adapters, though, in a multiple-database scenario. This commit modifies the patches so that they are applied only if the connection is SQL Server. If not, the original ActiveRecord implementation (`super`) is used instead. Fixes rails-sqlserver#929
37d9a67 to
22fb119
Compare
|
@wpolicarpo thanks for the feedback! I think I've addressed all of your concerns:
I also squashed this PR down to one commit. Let me know if there is anything else you need. |
|
All good. Thanks for your contribution. |
…#933) Recent versions of Rails support multiple database connections within the same app. It is possible for these connections to use different adapters. For example, one adapter may use SQL Server, and another uses PostgreSQL. This gem applies some monkey patches to ActiveRecord for SQL Server compatibility. These patches could break other adapters, though, in a multiple-database scenario. This commit modifies the patches so that they are applied only if the connection is SQL Server. If not, the original ActiveRecord implementation (`super`) is used instead. Fixes rails-sqlserver#929
Recent versions of Rails support multiple database connections within the same app. It is possible for these connections to use different adapters. For example, one adapter may use SQL Server, and another uses PostgreSQL.
This gem applies some monkey patches to ActiveRecord for SQL Server compatibility. These patches could break other adapters, though, in a multiple-database scenario.
This PR modifies the patches so that they are applied only if the connection is SQL Server. If not, the original ActiveRecord implementation (
super) is used instead.Fixes #929
@aidanharan @wpolicarpo what do you think of this approach? Any suggestions on how to test?