Opened 6 years ago

Closed 3 years ago

#10648 closed Cleanup/optimization (wontfix)

Django should warn when creating a model with field called "save".

Reported by: Thomas Steinacher <tom@…> Owned by: nobody
Component: Database layer (models, ORM) Version: 1.0
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The following model

class MyModel(models.Model):
    save = models.BooleanField(...)

causes error messages like

  File "django/db/models/query.py", line 373, in get_or_create
    obj.save(force_insert=True)

TypeError: 'bool' object is not callable

Django's ORM should give a more intuitive error message (e.g. in manage.py sql/syncdb) and not allow the user to have models with a field called "save".

Change History (7)

comment:1 Changed 6 years ago by jacob

  • milestone set to 1.1
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by jacob

  • Triage Stage changed from Accepted to Design decision needed

I'm not totally convinced that we should bother warning here -- at a certain point, we can't just put every single possible mistake into the validation warnings. We need to rely on users to do the right thing.

comment:3 Changed 6 years ago by jacob

  • milestone 1.1 deleted

comment:4 follow-up: Changed 6 years ago by mtredinnick

If we do this, we shouldn't allow fields to replace any methods that already exist. "save" isn't special. Tend to agree with Jacob, though: this is a standard thing throughout Python. You have to be careful not to override parent class attributes and Django shouldn't try to be a "wrapped in cotton wool" version of Python, even by replacing Python warts.

comment:5 in reply to: ↑ 4 Changed 6 years ago by Thomas Steinacher <tom@…>

If we do this, we shouldn't allow fields to replace any methods that already exist.

You're right on that one. I am not sure if it is even possible to check that. I leave it up to you to decide. It just took me more time than necessary to figure out why I get strange error messages.

comment:6 Changed 4 years ago by sethtrain

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:7 Changed 3 years ago by aaugustin

  • Easy pickings unset
  • Resolution set to wontfix
  • Status changed from new to closed
  • UI/UX unset

I'm closing this ticket for the reasons explained by Jacob and Malcolm above.

I don't believe it's worth trying to catch these errors at validation time, and I'm not even sure it's possible. How could we catch "metaclass = ...", since the Model metaclass won't even kick in?

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