Electron100 / butane

An ORM for Rust with a focus on simplicity and on writing Rust, not SQL
Apache License 2.0
94 stars 13 forks source link

state.saved #149

Closed jayvdb closed 11 months ago

jayvdb commented 11 months ago

There are two main problems with state.saved.

  1. When a transaction is rolled back, the saved flag is still true.
  2. If there are multiple copies of the object, and one is saved, and the other doesnt know it is saved. This is a similar problem as https://github.com/Electron100/butane/issues/112 , and since it is solved this isnt as high a priority as saving the second object wont fail any longer.

Also it requires adding a field to the structs being stored in the database - this isnt a problem so much as it adds a lot of complexity.

The state.saved flag was necessary before upsert support. It looks like it is still necessary for inserts into auto-pk tables (but maybe that can be worked around also?).

It is also used by the "dont save Many relationships linking to objects not yet persisted in the database" implementation at https://github.com/Electron100/butane/pull/116 , which IMO is a very good feature that I would hate to lose.

This issue is sort of a brain dump, with more investigation needed to see what direction that butane should take wrt this field. Maybe it is only more detailed documentation that is needed.

Electron100 commented 11 months ago

I've been unhappy with ObjectState for a while and had started taking a look at what removing it entirely would look like. I cleaned up that branch a bit today and put out a draft PR #151. Take a look and see what you think about the direction.

It does still preserve the ability to error on Many.save if referencing an auto-primary-key which hasn't been generated yet. It doesn't do any detection on non-automatic primary keys, although in that case the order of saving the Many vs. the object being referred to doesn't matter.

jayvdb commented 11 months ago

Fixed in #151