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 prepared statements support for Mysql2Adapter #23461

Merged
merged 1 commit into from
Apr 24, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ platforms :ruby, :mswin, :mswin64, :mingw, :x64_mingw do

group :db do
gem 'pg', '>= 0.18.0'
gem 'mysql2', '>= 0.4.0'
gem 'mysql2', '>= 0.4.4'
end
end

Expand Down
8 changes: 4 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,9 @@ GEM
mono_logger (1.1.0)
multi_json (1.11.2)
mustache (1.0.2)
mysql2 (0.4.2)
mysql2 (0.4.2-x64-mingw32)
mysql2 (0.4.2-x86-mingw32)
mysql2 (0.4.4)
mysql2 (0.4.4-x64-mingw32)
mysql2 (0.4.4-x86-mingw32)
nio4r (1.2.1)
nokogiri (1.6.7.2)
mini_portile2 (~> 2.0.0.rc2)
Expand Down Expand Up @@ -294,7 +294,7 @@ DEPENDENCIES
listen (~> 3.0.5)
minitest (< 5.3.4)
mocha (~> 0.14)
mysql2 (>= 0.4.0)
mysql2 (>= 0.4.4)
nokogiri (>= 1.6.7.1)
pg (>= 0.18.0)
psych (~> 2.0)
Expand Down
4 changes: 4 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
* Add prepared statements support for `Mysql2Adapter`.

*Ryuta Kamizono*

* Schema dumper: Indexes are now included in the `create_table` block
instead of listed afterward as separate `add_index` lines.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'active_record/connection_adapters/abstract_adapter'
require 'active_record/connection_adapters/statement_pool'
require 'active_record/connection_adapters/mysql/column'
require 'active_record/connection_adapters/mysql/explain_pretty_printer'
require 'active_record/connection_adapters/mysql/quoting'
Expand Down Expand Up @@ -56,9 +57,19 @@ def arel_visitor # :nodoc:
INDEX_TYPES = [:fulltext, :spatial]
INDEX_USINGS = [:btree, :hash]

class StatementPool < ConnectionAdapters::StatementPool
private

def dealloc(stmt)
stmt[:stmt].close
end
end

def initialize(connection, logger, connection_options, config)
super(connection, logger, config)

@statements = StatementPool.new(self.class.type_cast_config_to_integer(config.fetch(:statement_limit) { 1000 }))

if version < '5.0.0'
raise "Your version of MySQL (#{full_version.match(/^\d+\.\d+\.\d+/)[0]}) is too old. Active Record supports MySQL >= 5.0."
end
Expand Down Expand Up @@ -93,6 +104,12 @@ def supports_bulk_alter? #:nodoc:
true
end

# Returns true, since this connection adapter supports prepared statement
# caching.
def supports_statement_cache?
true
end

# Technically MySQL allows to create indexes with the sort order syntax
# but at the moment (5.5) it doesn't yet implement them
def supports_index_sort_order?
Expand Down Expand Up @@ -178,6 +195,14 @@ def disable_referential_integrity #:nodoc:
end
end

# CONNECTION MANAGEMENT ====================================

# Clears the prepared statements cache.
def clear_cache!
reload_type_map
Copy link
Member

Choose a reason for hiding this comment

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

Should we call super here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

👍

@statements.clear
end

#--
# DATABASE STATEMENTS ======================================
#++
Expand All @@ -191,11 +216,6 @@ def explain(arel, binds = [])
MySQL::ExplainPrettyPrinter.new.pp(result, elapsed)
end

def clear_cache!
super
reload_type_map
end

# Executes the SQL statement in the context of this connection.
def execute(sql, name = nil)
log(sql, name) { @connection.query(sql) }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
module ActiveRecord
module ConnectionAdapters
module MySQL
module DatabaseStatements
# Returns an ActiveRecord::Result instance.
def select_all(arel, name = nil, binds = [], preparable: nil)
result = if ExplainRegistry.collect? && prepared_statements
unprepared_statement { super }
else
super
end
@connection.next_result while @connection.more_results?
result
end

# Returns a record hash with the column names as keys and column values
# as values.
def select_one(arel, name = nil, binds = [])
arel, binds = binds_from_relation(arel, binds)
@connection.query_options.merge!(as: :hash)
Copy link
Member

Choose a reason for hiding this comment

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

What this does?

Copy link
Member Author

Choose a reason for hiding this comment

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

stmt.execute calls result.each in the internal.
https://github.com/brianmario/mysql2/blob/0.4.4/ext/mysql2/statement.c#L384
Need to set query_options before stmt.execute.

Copy link
Member

Choose a reason for hiding this comment

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

But how returning a hash instead of array in the result would be affected by the result.each call?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I see now. Thanks for the explanation!

select_result(to_sql(arel, binds), name, binds) do |result|
@connection.next_result while @connection.more_results?
result.first
end
ensure
@connection.query_options.merge!(as: :array)
end

# Returns an array of arrays containing the field values.
# Order is the same as that returned by +columns+.
def select_rows(sql, name = nil, binds = [])
select_result(sql, name, binds) do |result|
@connection.next_result while @connection.more_results?
result.to_a
end
end

# Executes the SQL statement in the context of this connection.
def execute(sql, name = nil)
if @connection
# make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been
# made since we established the connection
@connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone
end

super
end

def exec_query(sql, name = 'SQL', binds = [], prepare: false)
if without_prepared_statement?(binds)
execute_and_free(sql, name) do |result|
ActiveRecord::Result.new(result.fields, result.to_a) if result
end
else
exec_stmt_and_free(sql, name, binds, cache_stmt: prepare) do |_, result|
ActiveRecord::Result.new(result.fields, result.to_a) if result
end
end
end

def exec_delete(sql, name, binds)
if without_prepared_statement?(binds)
execute_and_free(sql, name) { @connection.affected_rows }
else
exec_stmt_and_free(sql, name, binds) { |stmt| stmt.affected_rows }
end
end
alias :exec_update :exec_delete

protected

def last_inserted_id(result)
@connection.last_id
end

private

def select_result(sql, name = nil, binds = [])
if without_prepared_statement?(binds)
execute_and_free(sql, name) { |result| yield result }
else
exec_stmt_and_free(sql, name, binds, cache_stmt: true) { |_, result| yield result }
end
end

def exec_stmt_and_free(sql, name, binds, cache_stmt: false)
if @connection
# make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been
# made since we established the connection
@connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone
end

type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) }

log(sql, name, binds) do
if cache_stmt
cache = @statements[sql] ||= {
stmt: @connection.prepare(sql)
}
stmt = cache[:stmt]
else
stmt = @connection.prepare(sql)
end

begin
result = stmt.execute(*type_casted_binds)
rescue Mysql2::Error => e
if cache_stmt
@statements.delete(sql)
else
stmt.close
end
raise e
end

ret = yield stmt, result
result.free if result
stmt.close unless cache_stmt
ret
end
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require 'active_record/connection_adapters/abstract_mysql_adapter'
require 'active_record/connection_adapters/mysql/database_statements'

gem 'mysql2', '>= 0.3.18', '< 0.5'
gem 'mysql2', '~> 0.4.4'
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to bump mysql2 minimum version? 0.4.x is only required if prepared_statements: true.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, mysql2 0.4.3 was broken without prepared statements.
I would like to skip this version.

require 'mysql2'

module ActiveRecord
Expand Down Expand Up @@ -35,9 +36,11 @@ module ConnectionAdapters
class Mysql2Adapter < AbstractMysqlAdapter
ADAPTER_NAME = 'Mysql2'.freeze

include MySQL::DatabaseStatements

def initialize(connection, logger, connection_options, config)
super
@prepared_statements = false
@prepared_statements = false unless config.key?(:prepared_statements)
Copy link
Member

Choose a reason for hiding this comment

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

Should we use the value of the prepared_statements config here like we do in the other adapters?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is for prepared_statements disabled by default (keep previous default).
Should we make to enabled by default?

Copy link
Member

Choose a reason for hiding this comment

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

We should respect it. If users ask to enable it, it should be enabled, to disable it, it should be disabled. This implementation while keeping the default is enabling if you do prepared_statements: false in your config.

Copy link
Member Author

Choose a reason for hiding this comment

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

This implementation is:

if config.key?(:prepared_statements)
  # @prepared_statements is configured in `super`
  super
else
  # keep the previous default (disabled) when `:prepared_statements` is not specified
  @prepared_statements = false
end

Copy link
Member

Choose a reason for hiding this comment

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

Oh. I missed that change that moved AbstractAdapter to configure prepared_statements 👍

configure_connection
end

Expand Down Expand Up @@ -103,55 +106,6 @@ def disconnect!
end
end

#--
# DATABASE STATEMENTS ======================================
#++

# Returns a record hash with the column names as keys and column values
# as values.
def select_one(arel, name = nil, binds = [])
arel, binds = binds_from_relation(arel, binds)
execute(to_sql(arel, binds), name).each(as: :hash) do |row|
@connection.next_result while @connection.more_results?
return row
end
end

# Returns an array of arrays containing the field values.
# Order is the same as that returned by +columns+.
def select_rows(sql, name = nil, binds = [])
result = execute(sql, name)
@connection.next_result while @connection.more_results?
result.to_a
end

# Executes the SQL statement in the context of this connection.
def execute(sql, name = nil)
if @connection
# make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been
# made since we established the connection
@connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone
end

super
end

def exec_query(sql, name = 'SQL', binds = [], prepare: false)
result = execute(sql, name)
@connection.next_result while @connection.more_results?
ActiveRecord::Result.new(result.fields, result.to_a) if result
end

def exec_delete(sql, name, binds)
execute to_sql(sql, binds), name
@connection.affected_rows
end
alias :exec_update :exec_delete

def last_inserted_id(result)
@connection.last_id
end

private

def connect
Expand Down