ActiveRecord: when exactly is a record (model) saved to the database?

Fred, example [1] was meant to be a minimal example. I am more concerned about a situation when the same record is accessed by two applications or through two objects.

I said that my question was philosophical (which does not mean that such behavior is not a bug). I understand why SQL executes "successfully", but if .save is only meant as a shortcut to a specific SQL, it should have been called differently, and in fact it should have been simply replaced by two methods: .insert and .update, which would return "true" if the SQL executed successfully, and there i wouldn't have been surprised.

The way it is, .save seems to hang awkwardly halfway between pure SQL (but not quite there because it verifies if the object has been saved before, which in many situation can be redundant, as the developer might know this in advance) and something more intelligent that would return "true" only if the object has been saved.

Any more comments, or should i try to submit a bug report?

Alexey.

Or.. make save work. Why should a deleted record be any different than a new unsaved record? When a record is destroyed it should lose its id and if saved again should insert a new row. At least, that's what I'd expect, if the principle of least surprise means anything.

Except in this case your “record” is an object instance. Since an object instance is not a pointer to the actual record in the database it should not change when you destroy that record. Keep in mind that there are many different ways to destroy a record in a database that don’t involve calling the .destroy method of the object instance. What should happen is that the .save method should return false if it can’t insert or update the record by that id. From following the thread it looks like its actually checking for an SQL execution error and it is not receiving one from the database. What its getting back is a successful execution of the SQL statement but a message that states the there was no record to create/update. From this information it then returns the incorrect state of true.

B.

Except in this case your "record" is an object instance.

Of course.

Since an object instance is not a pointer to the actual record in the database it should not change when you destroy that record.

That doesn't follow. If there is no longer a row 1 in the database, the object should no longer reference row 1. Currently the object is left in an invalid state and save thinks an update is possible when it isn't. The abstraction is leaking.

Keep in mind that there are many different ways to destroy a record in a database that don't involve calling the .destroy method of the object instance.

Not relevant to this case.

What should happen is that the .save method should return false if it can't insert or update the record by that id.

At a minimum, agreed; it should show the affected row count. But success is better than failure and you still haven't acknowledged that a destroyed object is conceptually identical to a new object that hasn't been saved and logically, should behave the same. Save could insert a new record and it would perfectly conceptually elegant and plug the leaking abstraction.

From following the thread it looks like its actually checking for an SQL execution error and it is not receiving one from the database. What its getting back is a successful execution of the SQL statement but a message that states the there was no record to create/update. From this information it then returns the incorrect state of true.

The current behavior is clearly wrong of course.

> So yes, it probably would make more sense to have .save return false > or raise on saving a destroyed record - but it should raise if you try > to alter a destroyed record, and it makes not much sense to save a > record you've destroyed and not altered... :-/

> I don't know which way to plump... any thoughts?

Or.. make save work. Why should a deleted record be any different than a new unsaved record? When a record is destroyed it should lose its id and if saved again should insert a new row. At least, that's what I'd expect, if the principle of least surprise means anything.

That sounds dangerous to me. If someone decided to destroy a record, resurrecting it because in a narrow window of opportunity someone tried to update it sounds like it would lead to very difficult to track down bugs. For save to fail/raise an exception would be less dangerous

Fred

I see your point. Without being able to modify the object after the .destroy call it becomes a read-only object of no value. However, being able to salvage that object as “new” one would solve the issue. I like it.

B.

surprise" principle would be to

1. implement .insert and .update methods which would simply execute SQL and return it's errors. 2. abandon the .destroyed? method and its @destroyed instance variable 3. switch .persisted? (@persisted) to false when the record is destroyed or if its .id is manually changed in the program 4. implement .save as something intelligent.

Of course such drastic changes might not seem practical...

Alexey.

I have created a ticket about this at Lighthouse, please comment:

https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/6666-save-activerecordpersistence-can-return-true-even-when-the-model-is-not-saved-and-seems-to-break-the-principle-of-least-surprise