[Django] #24894: Add CURRENT_TIMESTAMP function to contrib.postgres
Django
----------------------------------+------------------------
Reporter: adamchainz | Owner: adamchainz
Type: New feature | Status: new
Component: contrib.postgres | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------
#24866 implements a Now() function for the ORM. For postgres, it uses
STATEMENT_TIMESTAMP(), in order to be cross-compatible with the other
database backends, rather than CURRENT_TIMESTAMP - this is because
CURRENT_TIMESTAMP is the same across the transaction. Adding a
TransactionNow function to django.contrib.postgres would make this
(default) behaviour discoverable and easily usable with Django.
--
Ticket URL: <https://code.djangoproject.com/ticket/24894>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: adamchainz | Owner: adamchainz
Type: New feature | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Carl says:
I find `TransactionNow()` to be more useful than `Now()` in most cases I
run into (e.g. it's often useful for two objects created in the same
transaction to have the same creation timestamp). In other words, as usual
Postgres gets it right and the other databases get it wrong :-)
[[BR]][[BR]]
That said, `TransactionNow()` (like any other SQL function) is now
trivial to implement yourself, so I won't be bothered if it doesn't make
it into contrib.postgres. But on balance I'd be in favor.
Other opinions?
--
Ticket URL: <https://code.djangoproject.com/ticket/24894#comment:1>
Django
Reporter: adamchainz | Owner: adamchainz
Type: New feature | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by mjtamlyn):
I agree with Carl that postgres' transaction now is actually more helpful
than the statement now in most cases. Assuming other databases don't have
that feature, is there any reason not for us to be opinionated and use the
better PG implementation in the main `Now()` function added in #24866? In
most practical use cases they'll be milliseconds off anyway. I guess it's
the difference between:
{{{
now = timezone.now()
a = Foo.objects.create(created=now)
b = Foo.objects.create(created=now)
self.assertEqual(a.created, b.created)
}}}
and
{{{
a = Foo.objects.create(created=timezone.now())
b = Foo.objects.create(created=timezone.now())
self.assertNotEqual(a.created, b.created)
}}}
(Of course on older MySQL chances are they'd end up the same in the DB
anyway!)
I feel that having both in Django would be confusing and unnecessary.
Millisecond resolution of timestamp creation is a rare use case and most
sites wouldn't notice the difference if they changed database.
--
Ticket URL: <https://code.djangoproject.com/ticket/24894#comment:2>
Django
Reporter: adamchainz | Owner: adamchainz
Type: New feature | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by carljm):
I think the choice to keep `Now()` consistent was the right one.
Milliseconds might not seem like much, but the distinction between
"multiple objects created in one transaction have identical timestamp" vs
"multiple objects created in one transaction have very slightly different
timestamps" is a crucial difference in some cases, like if you're trying
to enforce unique timestamps, or trying to later query objects that were
created together, or whatever. I think potentially-critical semantic
differences like that should be reflected in the code, and we should avoid
making them inconsistent between database backends when possible.
As far as I'm concerned, a cross-db-consistent `Now()` plus a
`TransactionNow()` in `contrib.postgres` is pretty much the ideal
resolution here.
--
Ticket URL: <https://code.djangoproject.com/ticket/24894#comment:3>
Django
Reporter: adamchainz | Owner: adamchainz
Type: New feature | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by carljm):
One other thought - I would like to see this patch include some kind of
brief note in the `Now()` docs to alert Postgres users (who may be
accustomed to Postgres' `CURRENT_TIMESTAMP` behavior) that `Now()` is the
_statement_ timestamp on all databases, and direct them to
`TransactionNow()` if that's what they want.
--
Ticket URL: <https://code.djangoproject.com/ticket/24894#comment:4>
Django
Reporter: adamchainz | Owner: adamchainz
Type: New feature | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by mjtamlyn):
* stage: Unreviewed => Accepted
Comment:
Ok, I think I can agree with Carl here. Let's add `TransactionNow()` on
the condition we document the difference, and explain to non-pg users how
they can fudge `TransactionNow()` like behaviour using the first of my
examples above.
Another related question - does `default=Now()` on a model field work? I
imagine it may not as expressions are not deconstructible so it can't work
in migrations? Have expressions accidentally given us database level
default support, and if so can we use that for UUID pks?
--
Ticket URL: <https://code.djangoproject.com/ticket/24894#comment:5>
Django
Reporter: adamchainz | Owner: adamchainz
Type: New feature | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by carljm):
My guess is that expressions as defaults don't work - but at first glance
there doesn't seem to be a reason why they _couldn't_, and that would be
really nice for some cases. (Expressions may not be deconstructible, but
there's no reason they couldn't be.)
--
Ticket URL: <https://code.djangoproject.com/ticket/24894#comment:6>
Django
Reporter: adamchainz | Owner: adamchainz
Type: New feature | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by adamchainz):
Expressions as defaults don't work, they would need at least #24509 (Allow
Expressions when saving new models) to be done. I've had a look at it and
it's quite complicated.
I'll add the document pointers to the patch now, it was originally there
when this was part of the #24866 patch but I forgot to add it during
rebase.
--
Ticket URL: <https://code.djangoproject.com/ticket/24894#comment:7>
Django
Reporter: adamchainz | Owner:
| adamchainz
Type: New feature | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24894#comment:8>
Django
Reporter: adamchainz | Owner:
| adamchainz
Type: New feature | Status: closed
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"d34d39ade76e6b67299d8d88a7e5a2278a793dc3" d34d39ad]:
{{{
#!CommitTicketReference repository=""
revision="d34d39ade76e6b67299d8d88a7e5a2278a793dc3"
Fixed #24894 -- Added contrib.postgres.functions.TransactionNow
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24894#comment:9>