Ignore invalid PRIMARY KEY values in ActiveRecord::Persistence.delete method

[fatkodima]

When some invalid primary key values are passed to ActiveRecord::Persistence.delete method, ActiveRecord still perform a DELETE query even if it is not needed. For example,

irb(main):001> User.delete([])
  User Delete All (0.6ms)  DELETE FROM "users" WHERE 1=0
=> 0

So people usually add conditions around the call like User.delete(ids) if ids.compact.any? or User.delete(id) if id or just forget these conditions and unneeded queries are ran. But we can remove this burden from the user. Recent real-world example https://github.com/rails/solid_cache/blob/9be6755a2d8c8fd1c294912e1fbf6bbb69410c61/app/models/solid_cache/entry.rb#L64-L66

But, can NULL be a valid value and can we actually ignore it? I checked, and it turns out that primary keys cannot be NULL - ANSI standard says so, PostgreSQL, MySQL, SQL Server and Oracle Server conform to that. There is an exception from SQLite. In the docs it says that it allows NULLs due to some historical reasons, unless some conditions are met (one of them is explicit NOT NULL or table is created in STRICT mode, read the docs from the link for the details). But in rails, we explicitly set it NOT NULL

rails/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

Line 77 in ad1e443

primary_key: "integer PRIMARY KEY AUTOINCREMENT NOT NULL",

and have a strict mode (#45346). So, I think it is safe to ignore nils as values provided to #delete.

[nvasilevski]

I guess if someone has strong concerns about nil or an array of [nil] should be supported as a valid primary key value we could drop the compact and only focus on an empty array [] case because this seems to be an extremely common pattern Rails can handle internally

[byroot]

Just a minor thing, otherwise 👍

[byroot]

I think the compact is a bit much. I'm fine with just checking nil and empty arrays though, as I see how we can end up here on the happy path.

[fatkodima]

Yeah, makes sense! Changed that.

[byroot]

Thanks!