From a234669d63d8d1fc8ab5b872712dfa8ceab4257a Mon Sep 17 00:00:00 2001 From: zzak Date: Mon, 3 Jul 2023 16:55:41 +0900 Subject: [PATCH] Disable database prepared statements when query logs are enabled Fixes #48398 Prepared Statements and Query Logs are incompatible features due to query logs making every query unique. Co-authored-by: Jean Boussier --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/lib/active_record.rb | 3 +++ .../connection_adapters/abstract_adapter.rb | 2 +- activerecord/lib/active_record/railtie.rb | 1 + activerecord/test/cases/adapter_test.rb | 17 +++++++++++++++++ guides/source/configuring.md | 2 ++ railties/test/application/query_logs_test.rb | 13 +++++++++++++ 7 files changed, 43 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 6a27f8ac672f7..78fbf05a34ef9 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Disable database prepared statements when query logs are enabled + + Prepared Statements and Query Logs are incompatible features due to query logs making every query unique. + + *zzak, Jean Boussier* + * Support decrypting data encrypted non-deterministically with a SHA1 hash digest. This adds a new Active Record encryption option to support decrypting data encrypted diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 16aa05d9a98ed..b6085fd9a2d54 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -174,6 +174,9 @@ module Tasks autoload :SQLiteDatabaseTasks, "active_record/tasks/sqlite_database_tasks" end + singleton_class.attr_accessor :disable_prepared_statements + self.disable_prepared_statements = false + # Lazily load the schema cache. This option will load the schema cache # when a connection is established rather than on boot. If set, # +config.active_record.use_schema_cache_dump+ will be set to false. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 282df33c3d49b..7853abc304293 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -160,7 +160,7 @@ def initialize(config_or_deprecated_connection, deprecated_logger = nil, depreca @statements = build_statement_pool self.lock_thread = nil - @prepared_statements = self.class.type_cast_config_to_boolean( + @prepared_statements = !ActiveRecord.disable_prepared_statements && self.class.type_cast_config_to_boolean( @config.fetch(:prepared_statements) { default_prepared_statements } ) diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 7b3e48a45dd11..abcbaeb4009cd 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -396,6 +396,7 @@ class Railtie < Rails::Railtie # :nodoc: db_host: ->(context) { context[:connection].pool.db_config.host }, database: ->(context) { context[:connection].pool.db_config.database } ) + ActiveRecord.disable_prepared_statements = true if app.config.active_record.query_log_tags.present? ActiveRecord::QueryLogs.tags = app.config.active_record.query_log_tags diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 2c7dc9f7fc2b1..69c06bfcd1603 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -158,6 +158,23 @@ def test_not_specifying_database_name_for_cross_database_selects end end + unless in_memory_db? + def test_disable_prepared_statements + original_prepared_statements = ActiveRecord.disable_prepared_statements + db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary") + ActiveRecord::Base.establish_connection(db_config.configuration_hash.merge(prepared_statements: true)) + + assert_predicate ActiveRecord::Base.connection, :prepared_statements? + + ActiveRecord.disable_prepared_statements = true + ActiveRecord::Base.establish_connection(db_config.configuration_hash.merge(prepared_statements: true)) + assert_not_predicate ActiveRecord::Base.connection, :prepared_statements? + ensure + ActiveRecord.disable_prepared_statements = original_prepared_statements + ActiveRecord::Base.establish_connection :arunit + end + end + def test_table_alias def @connection.test_table_alias_length() 10; end class << @connection diff --git a/guides/source/configuring.md b/guides/source/configuring.md index b2b9851c39071..d6954791c8d63 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1345,6 +1345,8 @@ The default value depends on the `config.load_defaults` target version: Specifies whether or not to enable adapter-level query comments. Defaults to `false`. +NOTE: When this is set to `true` database prepared statements will be automatically disabled. + #### `config.active_record.query_log_tags` Define an `Array` specifying the key/value tags to be inserted in an SQL diff --git a/railties/test/application/query_logs_test.rb b/railties/test/application/query_logs_test.rb index d9b8565203dad..f47cb4d71d8cb 100644 --- a/railties/test/application/query_logs_test.rb +++ b/railties/test/application/query_logs_test.rb @@ -10,6 +10,7 @@ class QueryLogsTest < ActiveSupport::TestCase def setup build_app(multi_db: true) + add_to_config "config.active_record.sqlite3_production_warning = false" rails("generate", "scaffold", "Pet", "name:string", "--database=animals") app_file "app/models/user.rb", <<-RUBY class User < ActiveRecord::Base @@ -78,8 +79,20 @@ def app assert_includes ActiveRecord.query_transformers, ActiveRecord::QueryLogs end + test "disables prepared statements when enabled" do + add_to_config "config.active_record.query_log_tags_enabled = true" + + boot_app + + assert_predicate ActiveRecord, :disable_prepared_statements + end + test "controller and job tags are defined by default" do add_to_config "config.active_record.query_log_tags_enabled = true" + app_file "config/initializers/active_record.rb", <<-RUBY + raise "Expected prepared_statements to be enabled" unless ActiveRecord::Base.connection.prepared_statements + ActiveRecord::Base.connection.execute("SELECT 1") + RUBY boot_app