On Fri, Apr 8, 2022 at 3:27 PM Magnus Hagander <mag...@hagander.net> wrote:
> > > On Fri, Apr 8, 2022 at 3:23 PM Perry Smith <p...@easesoftware.com> wrote: > >> >> >> On Apr 8, 2022, at 07:47, Jan Wieck <j...@wi3ck.info> wrote: >> >> On 4/8/22 01:57, Nikolay Samokhvalov wrote: >> >> On Thu, Apr 7, 2022 at 8:10 AM Jan Wieck <j...@wi3ck.info < >> mailto:j...@wi3ck.info <j...@wi3ck.info>>> 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. > > Oh, I should add. The code is correct. The name of the function is wrong, because it doesn't actually disable referential integrity. It only postpones it. -- Magnus Hagander Me: https://www.hagander.net/ <http://www.hagander.net/> Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
