On Fri, Apr 8, 2022 at 3:23 PM Perry Smith <pedz@xxxxxxxxxxxxxxxx> wrote:
On Apr 8, 2022, at 07:47, Jan Wieck <jan@xxxxxxxxxx> wrote:On 4/8/22 01:57, Nikolay Samokhvalov wrote:On Thu, Apr 7, 2022 at 8:10 AM Jan Wieck <jan@xxxxxxxxxx <mailto:jan@xxxxxxxxxx>> wrote:
So **IF** Active Record is using that feature, then it can dump any
amount of garbage into your PostgreSQL database and PostgreSQL will
happily accept it with zero integrity checking.
It's DISABLE TRIGGER ALL https://github.com/rails/rails/blob/831031a8cec5bfe59ef653ae2857d4fe64c5698d/activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb#L12 <https://github.com/rails/rails/blob/831031a8cec5bfe59ef653ae2857d4fe64c5698d/activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb#L12>
Similar poison, same side effect.
Looking further at that code it also directly updates the PostgreSQL system catalog. This is a big, red flag.
Why do the Rails developers think they need a sledgehammer like that? It seems to be doing that for over 7 years, so it is hard to tell from the commit log why they need to disable RI at all.It has been a long time since I’ve done Rails stuff. What follows is the best I can recall but please take it with a grain of salt.The first problem is that generally Rails does not put constraints in the database. There were others like me who thought that was insane and would put constraints in the database — this includes foreign key constraints, check constraints, etc. About the only constraint that could be added into the DB using native Rails was the “not null” constraint.When foreign and other constraints were added, it broke something they call “Fixtures” which are present db states that are plopped into the DB during testing. To “fix” that, I (and others) would add this into our code base: (I’m adding this to see what you guys think of it — is it safer / better or just as insane?)def disable_referential_integrity(&block)
transaction {
begin
execute "SET CONSTRAINTS ALL DEFERRED"
yield
ensure
execute "SET CONSTRAINTS ALL IMMEDIATE"
end
}
end
This is perfectly normal code and nothing wrong with it. DEFERRED constraints are how you are *supposed* to handle such things. It defers the check of the foreign key to the end of the transaction, but it will still fail to commit if the foreign key is broken *at that point*. But it lets you do things like modify multiple tables that refer to each other, and have the changes only checked when they're all done.
