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

Avoid setting notice receiver on PG connection when ignoring SQL warnings #47518

Merged
merged 1 commit into from Feb 27, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -189,8 +189,6 @@ def suppress_composite_primary_key(pk)
end

def handle_warnings(sql)
return if ActiveRecord.db_warnings_action.nil?

@notice_receiver_sql_warnings.each do |warning|
next if warning_ignored?(warning)

Expand Down
Expand Up @@ -950,11 +950,13 @@ def configure_connection
self.client_min_messages = @config[:min_messages] || "warning"
self.schema_search_path = @config[:schema_search_path] || @config[:schema_order]

@raw_connection.set_notice_receiver do |result|
message = result.error_field(PG::Result::PG_DIAG_MESSAGE_PRIMARY)
code = result.error_field(PG::Result::PG_DIAG_SQLSTATE)
level = result.error_field(PG::Result::PG_DIAG_SEVERITY)
@notice_receiver_sql_warnings << SQLWarning.new(message, code, level)
unless ActiveRecord.db_warnings_action.nil?
@raw_connection.set_notice_receiver do |result|
message = result.error_field(PG::Result::PG_DIAG_MESSAGE_PRIMARY)
code = result.error_field(PG::Result::PG_DIAG_SQLSTATE)
level = result.error_field(PG::Result::PG_DIAG_SEVERITY)
@notice_receiver_sql_warnings << SQLWarning.new(message, code, level)
end
end

# Use standard-conforming strings so we don't have to do the E'...' dance.
Expand Down
104 changes: 51 additions & 53 deletions activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
Expand Up @@ -520,93 +520,77 @@ def test_only_check_for_insensitive_comparison_capability_once
end

def test_ignores_warnings_when_behaviour_ignore
ActiveRecord.db_warnings_action = :ignore
with_db_warnings_action(:ignore) do
result = @connection.execute("do $$ BEGIN RAISE WARNING 'foo'; END; $$")

result = @connection.execute("do $$ BEGIN RAISE WARNING 'foo'; END; $$")

assert_equal [], result.to_a
ensure
ActiveRecord.db_warnings_action = @original_db_warnings_action
assert_equal [], result.to_a
end
end

def test_logs_warnings_when_behaviour_log
ActiveRecord.db_warnings_action = :log

sql_warning = "[ActiveRecord::SQLWarning] PostgreSQL SQL warning (01000)"
with_db_warnings_action(:log) do
sql_warning = "[ActiveRecord::SQLWarning] PostgreSQL SQL warning (01000)"

assert_called_with(ActiveRecord::Base.logger, :warn, [sql_warning]) do
@connection.execute("do $$ BEGIN RAISE WARNING 'PostgreSQL SQL warning'; END; $$")
assert_called_with(ActiveRecord::Base.logger, :warn, [sql_warning]) do
@connection.execute("do $$ BEGIN RAISE WARNING 'PostgreSQL SQL warning'; END; $$")
end
end
ensure
ActiveRecord.db_warnings_action = @original_db_warnings_action
end

def test_raises_warnings_when_behaviour_raise
ActiveRecord.db_warnings_action = :raise

assert_raises(ActiveRecord::SQLWarning) do
@connection.execute("do $$ BEGIN RAISE WARNING 'PostgreSQL SQL warning'; END; $$")
with_db_warnings_action(:raise) do
assert_raises(ActiveRecord::SQLWarning) do
@connection.execute("do $$ BEGIN RAISE WARNING 'PostgreSQL SQL warning'; END; $$")
end
end
ensure
ActiveRecord.db_warnings_action = @original_db_warnings_action
end

def test_reports_when_behaviour_report
ActiveRecord.db_warnings_action = :report
with_db_warnings_action(:report) do
error_reporter = ActiveSupport::ErrorReporter.new
subscriber = ActiveSupport::ErrorReporter::TestHelper::ErrorSubscriber.new

error_reporter = ActiveSupport::ErrorReporter.new
subscriber = ActiveSupport::ErrorReporter::TestHelper::ErrorSubscriber.new
Rails.define_singleton_method(:error) { error_reporter }
Rails.error.subscribe(subscriber)

Rails.define_singleton_method(:error) { error_reporter }
Rails.error.subscribe(subscriber)

@connection.execute("do $$ BEGIN RAISE WARNING 'PostgreSQL SQL warning'; END; $$")
warning_event, * = subscriber.events.first
@connection.execute("do $$ BEGIN RAISE WARNING 'PostgreSQL SQL warning'; END; $$")
warning_event, * = subscriber.events.first

assert_kind_of ActiveRecord::SQLWarning, warning_event
assert_equal "PostgreSQL SQL warning", warning_event.message
ensure
Rails.singleton_class.remove_method(:error)
ActiveRecord.db_warnings_action = @original_db_warnings_action
assert_kind_of ActiveRecord::SQLWarning, warning_event
assert_equal "PostgreSQL SQL warning", warning_event.message
end
end

def test_warnings_behaviour_can_be_customized_with_a_proc
warning_message = nil
warning_level = nil
ActiveRecord.db_warnings_action = ->(warning) do
warning_action = ->(warning) do
warning_message = warning.message
warning_level = warning.level
end

@connection.execute("do $$ BEGIN RAISE WARNING 'PostgreSQL SQL warning'; END; $$")
with_db_warnings_action(warning_action) do
@connection.execute("do $$ BEGIN RAISE WARNING 'PostgreSQL SQL warning'; END; $$")

assert_equal "PostgreSQL SQL warning", warning_message
assert_equal "WARNING", warning_level
ensure
ActiveRecord.db_warnings_action = @original_db_warnings_action
assert_equal "PostgreSQL SQL warning", warning_message
assert_equal "WARNING", warning_level
end
end

def test_allowlist_of_warnings_to_ignore
old_ignored_warnings = ActiveRecord.db_warnings_ignore
ActiveRecord.db_warnings_action = :raise
ActiveRecord.db_warnings_ignore = [/PostgreSQL SQL warning/]
with_db_warnings_action(:raise, [/PostgreSQL SQL warning/]) do
result = @connection.execute("do $$ BEGIN RAISE WARNING 'PostgreSQL SQL warning'; END; $$")

result = @connection.execute("do $$ BEGIN RAISE WARNING 'PostgreSQL SQL warning'; END; $$")

assert_equal [], result.to_a
ensure
ActiveRecord.db_warnings_action = @original_db_warnings_action
ActiveRecord.db_warnings_ignore = old_ignored_warnings
assert_equal [], result.to_a
end
end

def test_does_not_raise_notice_level_warnings
ActiveRecord.db_warnings_action = :raise
with_db_warnings_action(:raise, [/PostgreSQL SQL warning/]) do
result = @connection.execute("DROP TABLE IF EXISTS non_existent_table")

result = @connection.execute("DROP TABLE IF EXISTS non_existent_table")

assert_equal [], result.to_a
ensure
ActiveRecord.db_warnings_action = @original_db_warnings_action
assert_equal [], result.to_a
end
end

private
Expand All @@ -618,6 +602,20 @@ def connection_without_insert_returning
db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary")
ActiveRecord::Base.postgresql_connection(db_config.configuration_hash.merge(insert_returning: false))
end

def with_db_warnings_action(action, warnings_to_ignore = [])
original_db_warnings_ignore = ActiveRecord.db_warnings_ignore

ActiveRecord.db_warnings_action = action
ActiveRecord.db_warnings_ignore = warnings_to_ignore
@connection.disconnect! # Disconnect from the db so that we reconfigure the connection

yield
ensure
ActiveRecord.db_warnings_action = @original_db_warnings_action
ActiveRecord.db_warnings_ignore = original_db_warnings_ignore
@connection.disconnect!
end
end
end
end