Opened 13 years ago
Closed 9 years ago
#16745 closed Bug (wontfix)
Different times on fields with auto_now and auto_now_add
Reported by: | Owned by: | Michael Mior | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.3 |
Severity: | Normal | Keywords: | auto_now, auto_now_add, DateTimeField, DateField, field |
Cc: | kantntreiber@…, Michael Mior | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
There is a little issue with using "auto_now" and "auto_now_add" together in one model. Because the time is set by calling datetime.now() in the pre_save method of the field, using together more than one such field would lead to different values for every field. This is bad, as one expects that the time is the time of the creation or change of the object and therefore should be identical across all of its fields.
An example:
class FooModel(models.Model): time_changed = models.DateTimeField(auto_now=True) time_created = models.DateTimeField(auto_now_add=True) In [3]: from models import FooModel In [4]: o=FooModel() In [5]: o.time_changed In [6]: o.save() In [7]: o.time_changed Out[7]: datetime.datetime(2011, 9, 2, 11, 21, 37, 582956) In [8]: o.time_created Out[8]: datetime.datetime(2011, 9, 2, 11, 21, 37, 582857)
Attachments (2)
Change History (11)
comment:1 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 13 years ago
Cc: | added |
---|---|
Has patch: | set |
Needs documentation: | set |
Owner: | changed from | to
Patch needs improvement: | set |
Apologies for the poorly named initial patch attachment. (Can't seem to delete.) This is my first time attempting a contribution to Django. I attached a patch which is working for me. However, I'm not quite sure how this interacts with the newly-introduced timezone-aware times.
I noticed for DateField and TimeField, the current date/time is derived from datetime (i.e. datetime.date.today()
or datetime.datetime.now().time()
), but for DateTimeField, the time comes from timezone.now()
. Is this a bug also, or is there a reason for this I'm not aware of.
Anyway, the patch likely needs some work since I'm not too familiar with the Django core, but I'm happy to help :)
by , 13 years ago
Attachment: | auto-use-transaction-time.diff added |
---|
comment:3 by , 13 years ago
DateFields and TimeFiles are naive – they don't store timezone information – while DateTimeField is aware.
You should convert timezone.now() to the default time zone before extracting the date or time part.
comment:4 by , 13 years ago
I will try to get some time reviewing this patch. Recording the transaction start time reliably could be surprisingly hard, as generally Django has pretty bad knowledge of when in transaction and when not. But it seems the proposed patch will work.
I would like it even more if there were some way to get the actual database transaction start time ("SELECT now()" in PostgreSQL). But even as is, I think this patch has merit.
comment:5 by , 13 years ago
@aaugustin Would I be correct in understanding that django.utils.timezone.localtime
performs the conversion to the default time zone?
@kaaria Agreed that the transaction start time may be unreliable, but it should be roughly accurate. As far as I can tell, Django has no guarantees on the delta of these times from their actual values anyway. (Although perhaps calling the time transaction_start
is a bit of a misnomer.) My concern with using NOW()
from the database end is that it requires knowledge of the server's configured time zone, which I don't believe Django currently has. Correct me if I'm wrong.
comment:6 by , 13 years ago
In the case of PostgreSQL, the server configured time zone doesn't matter, the time zone used in the connection is what matters. And Django knows that.
But I might be pushing this a bit too far, maybe using database.now() should be a separate field option, and done in separate ticket. I am not sure if transaction start time is easily available when using other databases.
It seems I will be really busy at least until weekend. Have to see if I can find time to review this.
comment:7 by , 13 years ago
I suppose it could be helpful for debugging purposes and correlating logs. And maybe especially so in the case where one needs to integrate with legacy systems. The ability to use database time could be a plus. Although it seems like other parts for the core would need to be changed to have a consistent view of time. Actually. perhaps it would be easiest to allow TIME_ZONE
to be configured such that it will use the DB server's TZ. In any case, this feels outside the scope of this issue, but I'm open to continuing work on something along these lines if it proves useful.
comment:8 by , 9 years ago
Maybe it's worth closing this ticket now that we can use database functions, e.g. default=NOW
. I think auto_now
and auto_now_add
are somewhat discouraged these days (#22995).
comment:9 by , 9 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Accepted because, yes, it would be nice if the times matched. However I believe this will not get fixed soon, as there isn't a trivial (as in one-liner) fix, and for most developers this doesn't have a high priority. Unless of course somebody interested in fixing this creates a clean patch with tests.
To get me interested in this ticket, an approach which would go along the lines of:
In my opinion the approach is a good one because transaction start time has other uses than solving this ticket's problem, and it would be hopefully easy to solve this ticket's problem as a trivial one-liner as a side effect of having the transaction start time available. In addition, the transaction start time would make all objects saved in one transaction have the same time.
Then again, I don't know if core developers would be interested in this approach...