Code

Opened 5 months ago

Closed 4 months ago

#21478 closed Cleanup/optimization (fixed)

Documentation of field.db_type() wrong

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

Description

The docs (at https://docs.djangoproject.com/en/dev/howto/custom-model-fields/#django.db.models.Field.db_type) claim that the db_type method is only called on DB creation - however that is wrong, and has been wrong since 2008. The db_type method is called from WHERE clause construction, too.

Attachments (0)

Change History (6)

comment:1 Changed 5 months ago by schinckel

I'm working on a documentation patch.

However, it's not completely obvious to me how this should be cached. I mean, we could store it in the django cache, but that is implying that the django cache has been correctly set up. Most people will have set up the cache (as per https://docs.djangoproject.com/en/dev/topics/cache/#setting-up-the-cache), but what will happen if these settings have not been configured, and we attempt to store something in the cache?

Am I correct in assuming that this is something that would belong in the low-level cache, as the memoize decorator would only cache per instance of the field class (which gets recreated lots)?

Also, I'm not completely sure how to describe where it's being used. It's in the Constraint.process method, which indicates it "Returns a tuple of data suitable for inclusion in a WhereNode instance". What does this mean in high-level ORM-speak, or should I just replace the whole paragraph with something that indicates "This may be executed while preparing a query, so should be cached" or equivalent? I think this may be better, as the field class author will not know how the field may be used in the end.

comment:2 Changed 4 months ago by anubhav9042

Please see the pull request:
https://github.com/django/django/pull/2094

comment:3 Changed 4 months ago by timo

  • Has patch set
  • Needs documentation unset
  • Patch needs improvement set

Don't forget to set the trac flags appropriate to put the ticket in the review queue. I've marked "Has patch" and also "Patch needs improvement" per my comments on the PR. Thanks!

comment:4 Changed 4 months ago by anubhav9042

Changes to pull requests done.

comment:5 Changed 4 months ago by timo

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

Looks good to me, marking as RFC so @akaariai can take a look before it gets committed.

comment:6 Changed 4 months ago by Anubhav Joshi <anubhav9042@…>

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

In 1a9f13df59e83eb6e0c20f96d878826519eca62a:

Fixed #21478 -- Corrected docs for when Field.db_type() is called.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.