Opened 8 years ago

Closed 4 years ago

#5309 closed (fixed)

[Patch] Marking instances as db-stored via is_stored

Reported by: David Cramer <dcramer@…> Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: is_stored feature
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:

Description

This patch adds a flag to instance called 'is_stored'. By default it is set to None. When a DB instance is available in the database it is True, and when its removed it is False.

This corrects the need for Django guessing based on the PK value if it needs to try to do an update or insert, and should throw exceptions if you try to do an insert (.create) when a matching pk exists.

Attachments (4)

base.py.diff (2.1 KB) - added by David Cramer <dcramer@…> 8 years ago.
query.py.diff (1.3 KB) - added by David Cramer <dcramer@…> 8 years ago.
base.py.2.diff (2.1 KB) - added by David Cramer <dcramer@…> 8 years ago.
query.py.2.diff (1.3 KB) - added by David Cramer <dcramer@…> 8 years ago.

Download all attachments as: .zip

Change History (16)

Changed 8 years ago by David Cramer <dcramer@…>

Changed 8 years ago by David Cramer <dcramer@…>

Changed 8 years ago by David Cramer <dcramer@…>

Changed 8 years ago by David Cramer <dcramer@…>

comment:1 Changed 8 years ago by David Cramer <dcramer@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Fixed diff's, and changed is_stored to _is_stored.

comment:2 Changed 8 years ago by anonymous

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set

For future reference - use one patch, not two, and you won't win any friends without tests :-)

comment:3 Changed 8 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 7 years ago by adrian

  • Triage Stage changed from Design decision needed to Accepted

I like this a lot -- it's a smart optimization.

comment:5 Changed 7 years ago by adrian

  • Triage Stage changed from Accepted to Design decision needed

Hmm, I just realized there's a wide-open race condition here -- if an instance is marked as is_stored, then is deleted by another process, the save() method will be fooled into doing an UPDATE, and no records will be updated.

comment:6 Changed 7 years ago by PhiR

  • Keywords feature added

comment:7 Changed 7 years ago by dcramer

Design decision would be really good now. This isn't a feature, its a bug fix :)

comment:8 Changed 7 years ago by emulbreh

-1

You would also have to track primary key changes to clear _is_stored, which doesn't seem worth it given that you never know whether the object is still in the db (see adrian's comment).

Explicit update/insert support would be more useful to optimize/prevent bugs where it is required.

comment:9 follow-up: Changed 7 years ago by dcramer

Even if the primary key is changed it doesn't change the fact that it's stored. Are you saying that when you change the primary key of a row it should create a new row? (Thinking about it, I'd bet Django doesn't handle this, but it should).

comment:10 in reply to: ↑ 9 Changed 7 years ago by emulbreh

Replying to dcramer:

Even if the primary key is changed it doesn't change the fact that it's stored. Are you saying that when you change the primary key of a row it should create a new row? (Thinking about it, I'd bet Django doesn't handle this, but it should).

No, django doesn't handle changing pks via .save() - and it would be quite painful: you had to update all related objects as well. IMO, changing primary keys is mostly unnecessary. So yes, I'd say creating a new row (or updating an existent row) is the right thing to do when you change the pk.

comment:11 Changed 5 years ago by (removed)

  • Cc ferringb@… removed

comment:12 Changed 4 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

This seems to be solved via the addition of force_update and force_insert. That saves the guessing because you already know how you got the object.

Note: See TracTickets for help on using tickets.
Back to Top