Opened 3 months ago
Last modified 4 weeks ago
#35731 closed Cleanup/optimization
Extend documentation about db_default and DatabaseDefault — at Version 3
Reported by: | Kyle Bebak | Owned by: | YashRaj1506 |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Kyle Bebak, Simon Charette, Lily Foote | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
I would be helpful if the existing docs at ref/models/fields.txt
, when describing db_default
, would mention DatabaseDefault
.
Also, the docs topics/db/models.txt
describe most of the Field
options but db_default
is missing, so ideally we would add a section for it with examples, including some that would show how and when DatabaseDefault
is returned/used.
Original report
For example, if client code creates a model Foo
with val = IntegerField(db_default=10)
, does foo = Foo()
, and accesses foo.val
, they get an instance of django.db.models.expressions.DatabaseDefault
.
This DatabaseDefault
seems to be used for bookkeeping until the model instance is written to the DB, after which foo.val
is an int
. IMO this is not a good design, because it's a case of an implementation detail (setting a value for the field once it's saved to the DB) changing the model's public interface (IMO a model instance's field values are part of its public interface).
If instead we do val = IntegerField()
, and foo = Foo()
, and access foo.val
, we get None
, s.t. the type of foo.val
is int | None
. Using db_default
means that the type of foo.val
is now int | DatabaseDefault
. DatabaseDefault
is a bookkeeping type that client code usually shouldn't interact with. If users aren't aware of db_default
's implementation, they might still write code like this, which would be broken: if foo.val is not None: print(foo.val + 10)
.
Because DatabaseDefault
is for bookkeeping, it seems like there's no reason the model instance couldn't store its DatabaseDefault
instances on a "private" field which wouldn't affect the model's public interface. This would be a lot cleaner IMO. Most users shouldn't know about DatabaseDefault
, which unsurprisingly isn't mentioned here, https://docs.djangoproject.com/en/5.1/ref/models/fields/#db-default, or anywhere else in the docs AFAICT.
Change History (3)
comment:2 by , 3 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:3 by , 3 months ago
Description: | modified (diff) |
---|---|
Summary: | Fields with db_default, and without default, are initialized to instance of DatabaseDefault → Extend documentation about db_default and DatabaseDefault |
Triage Stage: | Unreviewed → Accepted |
Version: | 5.0 → dev |
Overall, I agree with Simon analysis. Specifically, the docs for db_default
say:
The database-computed default value for this field.
So, when doing val = IntegerField(db_default=10); foo = Foo(); foo.val
, I would expect anything but the value that was set as db_default
(10), since the code "hasn't gone to the db yet". Intuitively I would have expected an exception such as "you can't have a value since DB hasn't been reached yet", but getting a instance of a "db value promise" is even better and clearer.
OTOH, I do agree that we may be lacking docs about this. In particular, I think we should:
- add a small note to the existing
ref/models/fields.txt
docs aboutdb_default
, and - add a richer section to the
topics/db/models.txt
for:attr:~Field.db_default
since most of theField
options are documented there butdb_default
is missing.
Accepting and re-purposing this ticket with that goal in mind.
I don't agree that this is a change to the model's public interface. Assigning
Expression
like objects to field attributes on model instances has been a documented pattern for a while. See the section about Updating attributes based on existing fields for example.A sentinel object has to be assigned to the attribute to distinguish from
None
until its persisted and I'm afraid there is no way around that. What exactly would you expect the model instance attribute accesses to return whendb_default=TransactionNow()
or any other expression that is not a simple literal value like in your example? I personally don't find that returning a sentinel object that denotes this field's value is meant to be assigned a database expression on creation a surprising behavior when considering non-literal default values.The documentation you linked mentions
Given your report is specific to model instance initialization and includes an example of a
db_default
composed of a literal that can be expressed asdefault
I believe that your expectations are met by defining your field asIntegerField(default=10, db_default=10)
. I suspect that this feature that was specifically designed for the use case you had in mind.I'll let others chime in as I'm unsure if this should be closed as invalid (as specifying
default
achieves what you're after) or accepted as a documentation improvement given documenting the use case for defining both was brought up during the review phase and didn't make the cut.