diff options
Diffstat (limited to 'activerecord')
12 files changed, 69 insertions, 20 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index efb3ffebaf..901f043544 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* Don't cache prepared statements containing an IN clause or a SQL literal, as + these queries will change often and are unlikely to have a cache hit. + + *Sean Griffin* + * Fix `rewhere` in a `has_many` association. Fixes #21955. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index dbacf56795..848aeb821c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -29,7 +29,17 @@ module ActiveRecord # Returns an ActiveRecord::Result instance. def select_all(arel, name = nil, binds = []) arel, binds = binds_from_relation arel, binds - select(to_sql(arel, binds), name, binds) + sql = to_sql(arel, binds) + if arel.is_a?(String) + preparable = false + else + preparable = visitor.preparable + end + if prepared_statements && preparable + select_prepared(sql, name, binds) + else + select(sql, name, binds) + end end # Returns a record hash with the column names as keys and column values @@ -67,7 +77,7 @@ module ActiveRecord # Executes +sql+ statement in the context of this connection using # +binds+ as the bind substitutes. +name+ is logged along with # the executed +sql+ statement. - def exec_query(sql, name = 'SQL', binds = []) + def exec_query(sql, name = 'SQL', binds = [], prepare: false) end # Executes insert +sql+ statement in the context of this connection using @@ -358,9 +368,12 @@ module ActiveRecord # Returns an ActiveRecord::Result instance. def select(sql, name = nil, binds = []) - exec_query(sql, name, binds) + exec_query(sql, name, binds, prepare: false) end + def select_prepared(sql, name = nil, binds = []) + exec_query(sql, name, binds, prepare: true) + end # Returns the last auto-generated ID from the affected table. def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 9b46df721a..402159ac13 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -1,5 +1,6 @@ require 'active_record/type' require 'active_support/core_ext/benchmark' +require 'active_record/connection_adapters/determine_if_preparable_visitor' require 'active_record/connection_adapters/schema_cache' require 'active_record/connection_adapters/sql_type_metadata' require 'active_record/connection_adapters/abstract/schema_dumper' diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index e139092659..926b9e6987 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -153,6 +153,7 @@ module ActiveRecord if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @prepared_statements = true + @visitor.extend(DetermineIfPreparableVisitor) else @prepared_statements = false end diff --git a/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb b/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb new file mode 100644 index 0000000000..06356014ee --- /dev/null +++ b/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb @@ -0,0 +1,18 @@ +module DetermineIfPreparableVisitor + attr_reader :preparable + + def accept(*) + @preparable = true + super + end + + def visit_Arel_Nodes_In(*) + @preparable = false + super + end + + def visit_Arel_Nodes_SqlLiteral(*) + @preparable = false + super + end +end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 6d5f31c6ca..42c4a14f00 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -142,7 +142,7 @@ module ActiveRecord super end - def exec_query(sql, name = 'SQL', binds = []) + 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) diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index b3894481cc..765cdf90d2 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -235,11 +235,11 @@ module ActiveRecord @client_encoding = ENCODINGS[result.rows.last.last] end - def exec_query(sql, name = 'SQL', binds = []) + def exec_query(sql, name = 'SQL', binds = [], prepare: false) if without_prepared_statement?(binds) result_set, affected_rows = exec_without_stmt(sql, name) else - result_set, affected_rows = exec_stmt(sql, name, binds) + result_set, affected_rows = exec_stmt(sql, name, binds, cache_stmt: prepare) end yield affected_rows if block_given? @@ -378,12 +378,12 @@ module ActiveRecord private - def exec_stmt(sql, name, binds) + def exec_stmt(sql, name, binds, cache_stmt: false) cache = {} type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } log(sql, name, binds) do - if binds.empty? + if binds.empty? || !cache_stmt stmt = @connection.prepare(sql) else cache = @statements[sql] ||= { @@ -399,7 +399,7 @@ module ActiveRecord # place when an error occurs. To support older MySQL versions, we # need to close the statement and delete the statement from the # cache. - if binds.empty? + if binds.empty? || !cache_stmt stmt.close else @statements.delete sql diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index 43e18ebb2b..0e0c0e993a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -156,8 +156,8 @@ module ActiveRecord end end - def exec_query(sql, name = 'SQL', binds = []) - execute_and_clear(sql, name, binds) do |result| + def exec_query(sql, name = 'SQL', binds = [], prepare: false) + execute_and_clear(sql, name, binds, prepare: prepare) do |result| types = {} fields = result.fields fields.each_with_index do |fname, i| diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0686b35425..236c067fd5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -198,6 +198,7 @@ module ActiveRecord @visitor = Arel::Visitors::PostgreSQL.new self if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @prepared_statements = true + @visitor.extend(DetermineIfPreparableVisitor) else @prepared_statements = false end @@ -553,16 +554,22 @@ module ActiveRecord FEATURE_NOT_SUPPORTED = "0A000" #:nodoc: - def execute_and_clear(sql, name, binds) - result = without_prepared_statement?(binds) ? exec_no_cache(sql, name, binds) : - exec_cache(sql, name, binds) + def execute_and_clear(sql, name, binds, prepare: false) + if without_prepared_statement?(binds) + result = exec_no_cache(sql, name, []) + elsif !prepare + result = exec_no_cache(sql, name, binds) + else + result = exec_cache(sql, name, binds) + end ret = yield result result.clear ret end def exec_no_cache(sql, name, binds) - log(sql, name, binds) { @connection.async_exec(sql, []) } + type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } + log(sql, name, binds) { @connection.async_exec(sql, type_casted_binds) } end def exec_cache(sql, name, binds) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 505a71720f..5fb34dbfac 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -89,6 +89,7 @@ module ActiveRecord if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @prepared_statements = true + @visitor.extend(DetermineIfPreparableVisitor) else @prepared_statements = false end @@ -231,15 +232,18 @@ module ActiveRecord end end - def exec_query(sql, name = nil, binds = []) + def exec_query(sql, name = nil, binds = [], prepare: false) type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } log(sql, name, binds) do # Don't cache statements if they are not prepared - if without_prepared_statement?(binds) + unless prepare stmt = @connection.prepare(sql) begin cols = stmt.columns + unless without_prepared_statement?(binds) + stmt.bind_params(type_casted_binds) + end records = stmt.to_a ensure stmt.close @@ -252,7 +256,7 @@ module ActiveRecord stmt = cache[:stmt] cols = cache[:cols] ||= stmt.columns stmt.reset! - stmt.bind_params type_casted_binds + stmt.bind_params(type_casted_binds) end ActiveRecord::Result.new(cols, stmt.to_a) diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb index 820d41e13b..722e2377c1 100644 --- a/activerecord/test/cases/adapters/postgresql/connection_test.rb +++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb @@ -127,7 +127,7 @@ module ActiveRecord def test_statement_key_is_logged bind = Relation::QueryAttribute.new(nil, 1, Type::Value.new) - @connection.exec_query('SELECT $1::integer', 'SQL', [bind]) + @connection.exec_query('SELECT $1::integer', 'SQL', [bind], prepare: true) name = @subscriber.payloads.last[:statement_name] assert name res = @connection.exec_query("EXPLAIN (FORMAT JSON) EXECUTE #{name}(1)") diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index f89a394f96..93e98ec872 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -168,7 +168,7 @@ class SchemaTest < ActiveRecord::PostgreSQLTestCase def test_raise_wraped_exception_on_bad_prepare assert_raises(ActiveRecord::StatementInvalid) do - @connection.exec_query "select * from developers where id = ?", 'sql', [[nil, 1]] + @connection.exec_query "select * from developers where id = ?", 'sql', [bind_param(1)] end end |