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

Ensure triggers are enabled when operation fails in PostgreSQL #50196

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

fatkodima
Copy link
Member

Fixes #50191.

Shortened repro script:

$ cd activerecord
$ ARCONN=postgresql bin/test test/cases/fixtures_test.rb test/cases/adapters/postgresql/deferred_constraints_test.rb -n "/^(?:FixturesTest#(?:test_bulk_insert_with_a_multi_statement_query_raises_an_exception_when_any_insert_fails)|PostgresqlDeferredConstraintsTest#(?:test_defer_constraints_with_specific_fk))$/" --seed 49401

When fixture test executes

def test_bulk_insert_with_a_multi_statement_query_raises_an_exception_when_any_insert_fails
require "models/aircraft"
assert_equal false, Aircraft.columns_hash["wheels_count"].null
fixtures = {
"aircraft" => [
{ "name" => "working_aircrafts", "wheels_count" => 2 },
{ "name" => "broken_aircrafts", "wheels_count" => nil },
]
}
assert_no_difference "Aircraft.count" do
assert_raises(ActiveRecord::NotNullViolation) do
ActiveRecord::Base.connection.insert_fixtures_set(fixtures)
end
end
end
insert_fixtire_set from line 126 uses disable_referential_integrity. An error is raised, but triggers are not enabled back in
def disable_referential_integrity # :nodoc:
original_exception = nil
begin
transaction(requires_new: true) do
execute(tables.collect { |name| "ALTER TABLE #{quote_table_name(name)} DISABLE TRIGGER ALL" }.join(";"))
end
rescue ActiveRecord::ActiveRecordError => e
original_exception = e
end
begin
yield
rescue ActiveRecord::InvalidForeignKey => e
warn <<-WARNING
WARNING: Rails was not able to disable referential integrity.
This is most likely caused due to missing permissions.
Rails needs superuser privileges to disable referential integrity.
cause: #{original_exception&.message}
WARNING
raise e
end
begin
transaction(requires_new: true) do
execute(tables.collect { |name| "ALTER TABLE #{quote_table_name(name)} ENABLE TRIGGER ALL" }.join(";"))
end
rescue ActiveRecord::ActiveRecordError
end
end
and so for all the other tests from PostgresqlDeferredConstraintsTest they are not enabled and any deferrable foreign keys are just disabled.

cc @yahonda

@byroot byroot merged commit 4cbdffe into rails:main Nov 28, 2023
4 checks passed
@fatkodima fatkodima deleted the ensure-enable-trigger-all branch November 28, 2023 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PostgresqlDeferredConstraintsTest fails at Rails CI
2 participants