Hi Aymeric,
Your message seems to be confusing the queryset API with the model-instance
API. Details below.
On Sunday 31 January 2016 21:24:17 Aymeric Augustin wrote:
>
> 1) Ignore some fields on updates
>
> Django already provides three ways to do this:
>
> MyModel.objects.defer(‘ignored_1’, ‘ignored_2’).save()
> MyModel.objects.only(‘included_1’, ‘included_2’).save()
> MyModel.objects.save(update_fields=[‘included_1’, ‘included_2’])
>
> While we’re there, I don’t know if there’s a reason why `update_fields`
> can’t be replaced with `only()`. Both appear to achieve the same effet and
> `only()` is shorter. Could we deprecate `update_fields`?
>
only() is a queryset method. save() is a model method. There is actually no
MyModel.objects.save(), just MyModel.objects.only('field1').get(...).save().
Adding only() to model instances makes very little sense (is it supposed to
defer fields which have already been fetched?).
>
> 2) Ignore some fields on inserts
>
> Since Django abstracts the insert/update distinction with its single API,
> `save()`, I’m reluctant to create insert_* / update_* APIs.
>
> I don’t know how `defer()` and `only()` behave on inserts.
They don't. Since they only apply to objects fetched from the database, they
can only have any effect on updates (and I'm not even sure they do that).
WRT the original request: Owais, your API now seems to have gone a bit beyond
the needs detailed in the ticket description. In detail:
1. I think the separation between "delegate" and "return" in field options is
artificial (I find even the separation between "on insert" and "on update"
questionable). Django models always read (and write) all the fields against the
database, unless specifically told otherwise, and when told otherwise, it is on
a per-query basis, not in field definitions. You don't have a "deferred" flag
on
any field, so it is odd to have an equivalent only for computed fields. Thus,
of
your field optinos, I'd use only the "is_delegated" flag -- perhaps I'd allow
it
to have, besides True and False, a value for "insert only" (I would have to
see an example where "update only" made sense before suggesting that too).
2. The name "delegated" is a bit odd. "db_computed" or "from_db" seem better
to me; "delegated" is just too abstract (doesn't say delegated to whom, and
there are many options).
3. Once the fields are marked as computed in the database, any attempt to set
them explicitly should either override the database calculation, or be an
error; regardless of the comments above on API for choosing the fields, your
example:
class MyModel(models.Model):
pytthon_ts = fields.DatetimeField()
db_ts = fields.DatetimeField(delegated=True)
# This will only update python_ts as db_ts is delegated to database
MyModel.objects.update(db_ts=now(), python_ts=now())
I wouldn't like that behavior; updating python_ts is sensible, erroring out is
sensible, quietly ignoring user input isn't. Also, disabling triggers on a
per-query basis is at best suspicious. As consequence from all these,
ignore_delegates() seems problematic.
4. This leaves us with a "hole" in the API: Current APIs let us pick fields to
retrieve when we're selecting (only() and defer()) and fields to save when
we're updating (save(update_fields=...)) but the new API gives us a way to
retrieve fields on saving, with no way to defer those. I'd handle that with a
new argument to save, say 'refresh_fields' -- if it is not given, all computed
fields are retrieved.
5. In very very general -- look at the use cases in the ticket. The first case,
database-defined defaults, was discussed on this list[1] and a different
solution was agreed upon (though I don't think it was implemented yet). It has
also been made a lot easier since the ticket was filed, because we now have
database functions which we didn't have back then. The other use-cases are for
fields whose values are always controlled in the database; so, you can make
this patch a lot simpler by dropping the option for a field to be updated from
both the database and the app.
Hope this helps,
(and like Aymeric, sorry I didn't find the time to react to this sooner),
Shai.
[1] https://groups.google.com/d/topic/django-developers/3mcro17Gb40/discussion
