Opened 6 months ago

Last modified 6 months ago

#23386 new Cleanup/optimization

Document that F expressions are reapplied each time an object is saved

Reported by: codefisher Owned by: nobody
Component: Documentation Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I would have thought these two expresses would be exactly the same, except the second would be more efficient.

A: some_model.some_field += 1
B: some_model.some_field = F('some_field') + 1

I hit some unexpected behaviour when, I did this

some_model.save()
# and somewhere far far away
some_model.save()

With A, the field is incremented once, as I expected. But with B, it gets incremented twice. I would have expected the F query to effect the next database call, and nothing more. It might be intended, but then it is not documented.

I could write some come to demonstrate this. I only noticed it because I was using an import script that was creating lots of objects, and I had A and B in the model's overridden save method. They were causing vastly different results.

Attachments (1)

diff.txt (840 bytes) - added by codefisher 6 months ago.
Adding the needed note into the documentation

Download all attachments as: .zip

Change History (6)

comment:1 Changed 6 months ago by codefisher

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 months ago by timgraham

  • Component changed from Database layer (models, ORM) to Documentation
  • Summary changed from Unexpected result when incrimenting field with F expression to Document that F expressions are reapplied each time an object is saved
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

It doesn't seem intuitive and should definitely be documented. Probably a good idea to recommend fetching the object from the database after saving in order to get the new value if you are still using the object after save().

comment:3 Changed 6 months ago by codefisher

You say "Probably a good idea to recommend fetching the object from the database after saving in order to get the new value if you are still using the object after save()."

That is already done, at least on the latest dev docs (see the note): https://docs.djangoproject.com/en/dev/ref/models/queries/#f-expressions

But the effect of calling save more than once is no where to be seen, and really not that expected.

Last edited 6 months ago by codefisher (previous) (diff)

Changed 6 months ago by codefisher

Adding the needed note into the documentation

comment:4 Changed 6 months ago by codefisher

Just thinking about this, I am really more inclined to say how it works should be changed. I would suggest that after save() is called, it would be like the object had been called with defer() on that field. I don't think it would be hard to implement, I don't know the internals of the ORM, but defer() is already there, so what ever it does could be used again.

I can't see it causing backwards compatibility changes, this is undocumented, and not very intuitive. I would even suggest that after saving, that having the field affected by the F() expression not giving a sensible value is a bug in itself, and should be fixed - by treating as though marked by defer().

But there is always a change of causing backwards compatibility problems, that would be very hard to track down. Which needs to be weighed against how many problems the current counter intuitive behaviour would cause.

comment:5 Changed 6 months ago by jarshwah

I would be inclined to call this a bug - so I don't think backwards compatibility would be much of an issue. It's totally unexpected (for me at least) that the F() "persists" across a model.save() call. When the model is saved, the attribute should be converted to the relevant field/descriptor, rather than maintaining the proxy of the F() expression. I have no idea how hard or easy this would be without looking into it though.

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