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

Add 'set_session:' param to database.yml for mysql and mysql2 adapters #8346

Merged
merged 1 commit into from
Dec 8, 2012

Conversation

sodabrew
Copy link
Contributor

mysql and mysql2 session parameters can be set in the new 'set_session:' parameter in database.yml. The key-value pairs of this hash will be sent in a 'SET key = value, ...' query on new database connections.

Consolidated the configure_connection methods from mysql and mysql2 into the abstract_mysql base class.

Example use case, database.yml:

production:
  adapter: mysql2
  database: foo_prod
  user: foo
  set_session:
    sql_mode: traditional
    default_week_format: 1

See http://dev.mysql.com/doc/refman/5.0/en/set-statement.html and http://dev.mysql.com/doc/refman/5.0/en/server-system-variables.html

I originally wanted to remove the new 'strict: boolean' parameter, and run sql_mode just from the set_session section, but turns out that strict mode also cleans up some column value behavior within the mysql adapters. So the intersection of strict and set_session sql_mode is that when strict is true, you better not override sql_mode to a setting that is less-strict -- however you can override it to a value that is more strict (e.g. TRADITIONAL includes strict-mode's STRICT_ALL_TABLES plus adds a few more strictness options that many people might like!).

else
"@@SESSION.#{k.to_s} = #{quote(v)}" unless v.nil?
end
end.reject(&:nil?).join(', ')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use compact?

@carlosantoniodasilva
Copy link
Member

Nice work @sodabrew 👍

@sodabrew
Copy link
Contributor Author

Thanks! If this change is accepted, would you take a backport to 3.2.x as well? I'd be able to remove some configure_connection monkey patches from my rails projects that made me very quesy :). Basically the same code minus strict mode.

@rafaelfranca
Copy link
Member

Since it is adding feature we can't backport to a stable branch

@sodabrew
Copy link
Contributor Author

Ok, one branch at a time, hope you'll accept this pull for Rails 4!

@rafaelfranca
Copy link
Member

It is good to me.

👍

@tenderlove @jeremy objections?

@jeremy
Copy link
Member

jeremy commented Nov 29, 2012

Nice. set_session is an odd name for database.yml, though. How about settings?

production:
  adapter: mysql2
  database: foo_prod
  settings:
    sql_mode: traditional
    default_week_format: 1

@sodabrew
Copy link
Contributor Author

It is specifically for SET @@SESSION.foo = bar, so I figured on naming it close to that. All other settings are at that second level right below production / development, so the word settings would be kind of an obfuscation there (A newbie might ask, "Why is there a setting called settings?" ).

Next up I'd port this to other database adapters, it's also SET SESSION in Pg: http://www.postgresql.org/docs/8.3/static/sql-set.html

Nevermind, SQLite PRAGMA is something different. Although in SQLite it's called PRAGMA... http://www.sqlite.org/pragma.html (Perhaps aliasing the set_session key as set_pragma or just pragma for SQLite.)

@sodabrew
Copy link
Contributor Author

sodabrew commented Dec 7, 2012

All done with mysql, mysql2, and postgresql AR adapters. What are my next steps for inclusion of this PR?

@steveklabnik
Copy link
Member

Well, it doesn't merge cleanly any more...

@sodabrew
Copy link
Contributor Author

sodabrew commented Dec 7, 2012

Just the CHANGELOG that's in conflict. I'll rebase to master.

@sodabrew
Copy link
Contributor Author

sodabrew commented Dec 7, 2012

Branch rebased!

@jeremy
Copy link
Member

jeremy commented Dec 7, 2012

I'd go for variables: too.

The fact that these are set for the duration of the session is less important than that these are database variables that you're setting. Their duration is implied - as long as you use the connection.

@sodabrew
Copy link
Contributor Author

sodabrew commented Dec 7, 2012

I'm down for variables: -- I'll update the PR this evening.

@jeremy
Copy link
Member

jeremy commented Dec 7, 2012

❤️

@sodabrew
Copy link
Contributor Author

sodabrew commented Dec 8, 2012

Variables is now variables!

@jeremy
Copy link
Member

jeremy commented Dec 8, 2012

Great! Please squash commits & rebase against master :)

in the new 'variables:' hash in each database config section in database.yml.
The key-value pairs of this hash will be sent in a 'SET key = value, ...'
query on new database connections.

The configure_connection methods from mysql and mysql2 into are
consolidated into the abstract_mysql base class.
@sodabrew
Copy link
Contributor Author

sodabrew commented Dec 8, 2012

Rebased and squashed!

jeremy added a commit that referenced this pull request Dec 8, 2012
Add `variables:` to database.yml for mysql, mysql2, and postgresql adapters
@jeremy jeremy merged commit 61b528e into rails:master Dec 8, 2012
@jeremy
Copy link
Member

jeremy commented Dec 8, 2012

Thanks again @sodabrew !

@sodonnel
Copy link

sodonnel commented Oct 1, 2013

Has anyone confirmed this feature works (its been a long day, I could be doing something silly!)? I am trying to use it to change my mysql connections to not be in strict mode and getting no luck, so I put a debug statement or two into the ActiveRecord code (abstract_mysql_adapter.rb):

    puts variables
    if strict_mode? && !variables.has_key?(:sql_mode)
      variables[:sql_mode] = 'STRICT_ALL_TABLES'
    end

    puts "after"
    puts variables

This prints out:

{"sql_mode"=>"", :sql_auto_is_null=>0, :wait_timeout=>2147483}
after
{"sql_mode"=>"", :sql_auto_is_null=>0, :wait_timeout=>2147483, :sql_mode=>"STRICT_ALL_TABLES"}

So because the key I set in the database.yml file is stored in the variables hash as a string, it does not match in the if statement, which is checking for a key which is a symbol. Then I end up with two values corresponding to sql_mode, and I'm guessing the last one will win when past to mysql.

My database.yml looks like:

test:
  adapter: mysql
  encoding: utf8
  reconnect: false
  database: testdb
  pool: 5
  username: root
  password:
  socket: /tmp/mysql.sock
  variables:
    sql_mode: ''

@sodabrew
Copy link
Contributor Author

sodabrew commented Oct 1, 2013

@sodonnel You should set strict: false instead of trying to blank the variable.

@sodonnel
Copy link

sodonnel commented Oct 1, 2013

@sodabrew - Thanks, that seems to work for my goal right now.

However, if I wanted to set SQL_MODE to 'Traditional' (or something other than STRICT_ALL_TABLES), I suspect it's not going to work.

@sodabrew
Copy link
Contributor Author

sodabrew commented Oct 1, 2013

Please try it. If it doesn't work as expected, please open a new bug and reference this issue number and ping my name in the body of the bug report.

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.

None yet

7 participants