Permalink
Browse files

Check if the SQL is not a prepared statement

When the adapter is with prepared statement disabled and the binds array
is not empty the connection adapter will try to set the binds values and
will fail. Now we are checking if the adapter has the prepared statement
disabled.

Fixes #12023

Conflicts:
	activerecord/CHANGELOG.md
	activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
	activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
  • Loading branch information...
1 parent 918a148 commit ecf3b0094ca472172752449c1eebf18772db3d7e @rafaelfranca rafaelfranca committed Aug 28, 2013
@@ -1,5 +1,11 @@
## unreleased ##
+* Fix inserts with prepared statements disabled.
+
+ Fixes #12023.
+
+ *Rafael Mendonça França*
+
* Setting a has_one association on a new record no longer causes an empty
transaction.
@@ -377,14 +377,14 @@ def delete_sql(sql, name = nil)
update_sql(sql, name)
end
- def sql_for_insert(sql, pk, id_value, sequence_name, binds)
- [sql, binds]
- end
+ def sql_for_insert(sql, pk, id_value, sequence_name, binds)
+ [sql, binds]
+ end
- def last_inserted_id(result)
- row = result.rows.first
- row && row.first
- end
+ def last_inserted_id(result)
+ row = result.rows.first
+ row && row.first
+ end
end
end
end
@@ -99,6 +99,7 @@ def initialize(connection, logger = nil, pool = nil) #:nodoc:
@query_cache_enabled = false
@schema_cache = SchemaCache.new self
@visitor = nil
+ @prepared_statements = false
end
def valid_type?(type)
@@ -443,6 +444,10 @@ def translate_exception(exception, message)
# override in derived class
ActiveRecord::StatementInvalid.new(message, exception)
end
+
+ def without_prepared_statement?(binds)
+ @prepared_statements || binds.empty?
@nixme
nixme Sep 18, 2013

Isn't this boolean check a bit off? i.e. if @prepared_statements is false, signifying no use of prepared statements, but binds has stuff, then it'll use a prepared statement anyways.

@rafaelfranca
rafaelfranca Sep 23, 2013 Ruby on Rails member

Yes. It was incorrect but was already fixed.

@nixme
nixme Sep 23, 2013

Ah, I see that. Sorry, was just backporting this single fix for our app and missed the follow up commits. Thanks!

+ end
end
end
end
@@ -165,6 +165,7 @@ def initialize(connection, logger, connection_options, config)
@quoted_column_names, @quoted_table_names = {}, {}
if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true })
+ @prepared_statements = true
@visitor = Arel::Visitors::MySQL.new self
else
@visitor = unprepared_visitor
@@ -283,7 +283,7 @@ def exec_query(sql, name = 'SQL', binds = [])
# always be empty, since the bind variables will have been already
# substituted and removed from binds by BindVisitor, so this will
# effectively disable prepared statement usage completely.
- if binds.empty?
+ if without_prepared_statement?(binds)
result_set, affected_rows = exec_without_stmt(sql, name)
else
result_set, affected_rows = exec_stmt(sql, name, binds)
@@ -135,8 +135,8 @@ def substitute_at(column, index)
def exec_query(sql, name = 'SQL', binds = [])
log(sql, name, binds) do
- result = binds.empty? ? exec_no_cache(sql, binds) :
- exec_cache(sql, binds)
+ result = without_prepared_statement?(binds) ? exec_no_cache(sql, binds) :
+ exec_cache(sql, binds)
types = {}
fields = result.fields
@@ -524,6 +524,7 @@ def initialize(connection, logger, connection_parameters, config)
super(connection, logger)
if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true })
+ @prepared_statements = true
@visitor = Arel::Visitors::PostgreSQL.new self
else
@visitor = unprepared_visitor
@@ -113,6 +113,7 @@ def initialize(connection, logger, config)
@config = config
if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true })
+ @prepared_statements = true
@visitor = Arel::Visitors::SQLite.new self
else
@visitor = unprepared_visitor
@@ -293,8 +294,8 @@ def pp(result) # :nodoc:
def exec_query(sql, name = nil, binds = [])
log(sql, name, binds) do
- # Don't cache statements without bind values
- if binds.empty?
+ # Don't cache statements if they are not prepared
+ if without_prepared_statement?(binds)
stmt = @connection.prepare(sql)
cols = stmt.columns
records = stmt.to_a
@@ -564,6 +564,14 @@ def test_comparison
assert_equal [topic_2, topic_1].sort, [topic_1, topic_2]
end
+ def test_create_without_prepared_statement
+ topic = Topic.connection.unprepared_statement do
+ Topic.create(:title => 'foo')
+ end
+
+ assert_equal topic, Topic.find(topic.id)
+ end
+
def test_comparison_with_different_objects
topic = Topic.create
category = Category.create(:name => "comparison")

0 comments on commit ecf3b00

Please sign in to comment.