Re: New deferred loading implementation (Was: [Review Request] Added support for database delegated fields (like AutoField))

[Ben Cole]

This is what I wanted to know. I am willing to take on starting this DEP, 
with the caveat I've not done one before, and it will almost certainly 
require many revisions.

On Tuesday, 8 November 2016 15:32:48 UTC, Marc Tamlyn wrote:
>
> I have fed a lot into this discussion in person and on Slack. I don't 
> understand all the ins and outs completely, but I feel that the situation 
> is sufficiently complex enough that we need to consider it all together. 
> The code was certainly very challenging when I tried to make a UUID field 
> use a DB generated value and "return properly" when used as a pk, hence the 
> substandard docs for it. I'm personally -0 on the readonly patch unless it 
> does fit into a genuine coherent design.
>
> I personally believe we do need a DEP, and it needs to have the following 
> attributes:
> - Discussion of all the problems - refresh, read only, database level 
> defaults, primary keys and the auto generated "id" field.
> - Clear reference to all previous attempts to solve part(s) of the problem
> - A clear plan as to how all the use cases will be solved by the proposed 
> design
> - A road map of how to build the pieces one ticket at a time. readonly may 
> fit into this, it may not, that depends on the plan.
>
> This will allow everyone, but especially the technical board, to 
> understand all the considerations.
>
> Hope that helps. I may be overcomplicating the problem, but I don't think 
> I am.
> Marc
>
> On 8 November 2016 at 13:59, Joachim Jablon  > wrote:
>
>> We were having a discussion on this matter on the Django under the Hood 
>> Slack channel, and there's a design decision that I think I can't take by 
>> myself.
>>
>> There are 2 related subject : the "readonly" part and the "auto_refresh" 
>> part. Readonly means that the database won't try to write, autorefresh 
>> means that when creating /updating an instance, the value will be fetched 
>> from the database automatically
>>
>> There was a debate on whether the readonly behaviour should imply 
>> auto_refresh, and there are cases that are good candidates for auto_refresh 
>> without readonly (autofields, serial fields etc.).
>>
>> What I'd suggest, if we can get an agreement on this, would be to define 
>> those 2 behaviours completely independently (possibly as 2 different PRs, 
>> each with their tests and docs and such).
>>
>> Auto_refresh could then be used for autofields with the quirk that some 
>> SQL DBs, will provide last_insert_rowid() (sqlite) or LAST_INSERT_ID() 
>> (mysql), while other use RETURNING (PGSQL / Oracle) allowing to use a 
>> single query.
>>
>> Readonly, on its side, would only be a simple independent feature and its 
>> behaviour would do what the title says, no more, no less.
>>
>> Do you think I need to do a DEP on this ?
>> If not, do you think it's ok if 
>> https://github.com/django/django/pull/7515 is changed to only do the 
>> readonly part, and we make another PR (and possibly ticket) for the 
>> "auto_refresh" part ?
>>
>> While it would be nice to have both landing at the same time on master, I 
>> feel there's nothing blocking if they land at separate time, even in 
>> separate versions if need be (the freeze on Django 1.11 is coming fast, 
>> IIRC).
>>
>> Are there usecases that I'm missing and that could not be expressed as a 
>> mix of readonly and auto_refresh ?
>>
>> Le lundi 7 novembre 2016 15:35:38 UTC+1, Aymeric Augustin a écrit :
>>>
>>> On 7 Nov 2016, at 13:44, Joachim Jablon  wrote: 
>>>
>>> > My own personal opinion: 1. refesh by default, add an argument to 
>>> "model_instance.save()" to opt-out. 2. readonly 
>>>
>>> I agree. 
>>>
>>> -- 
>>> Aymeric. 
>>>
>>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to django-develop...@googlegroups.com .
>> To post to this group, send email to django-d...@googlegroups.com 
>> .
>> Visit this group at https://groups.google.com/group/django-developers.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/3db52e5b-54db-4736-845b-58a22b6f015c%40googlegroups.com
>>  
>> 
>> .
>>
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/85b9

Re: New deferred loading implementation (Was: [Review Request] Added support for database delegated fields (like AutoField))

[Marc Tamlyn]

I have fed a lot into this discussion in person and on Slack. I don't
understand all the ins and outs completely, but I feel that the situation
is sufficiently complex enough that we need to consider it all together.
The code was certainly very challenging when I tried to make a UUID field
use a DB generated value and "return properly" when used as a pk, hence the
substandard docs for it. I'm personally -0 on the readonly patch unless it
does fit into a genuine coherent design.

I personally believe we do need a DEP, and it needs to have the following
attributes:
- Discussion of all the problems - refresh, read only, database level
defaults, primary keys and the auto generated "id" field.
- Clear reference to all previous attempts to solve part(s) of the problem
- A clear plan as to how all the use cases will be solved by the proposed
design
- A road map of how to build the pieces one ticket at a time. readonly may
fit into this, it may not, that depends on the plan.

This will allow everyone, but especially the technical board, to understand
all the considerations.

Hope that helps. I may be overcomplicating the problem, but I don't think I
am.
Marc

On 8 November 2016 at 13:59, Joachim Jablon  wrote:

> We were having a discussion on this matter on the Django under the Hood
> Slack channel, and there's a design decision that I think I can't take by
> myself.
>
> There are 2 related subject : the "readonly" part and the "auto_refresh"
> part. Readonly means that the database won't try to write, autorefresh
> means that when creating /updating an instance, the value will be fetched
> from the database automatically
>
> There was a debate on whether the readonly behaviour should imply
> auto_refresh, and there are cases that are good candidates for auto_refresh
> without readonly (autofields, serial fields etc.).
>
> What I'd suggest, if we can get an agreement on this, would be to define
> those 2 behaviours completely independently (possibly as 2 different PRs,
> each with their tests and docs and such).
>
> Auto_refresh could then be used for autofields with the quirk that some
> SQL DBs, will provide last_insert_rowid() (sqlite) or LAST_INSERT_ID()
> (mysql), while other use RETURNING (PGSQL / Oracle) allowing to use a
> single query.
>
> Readonly, on its side, would only be a simple independent feature and its
> behaviour would do what the title says, no more, no less.
>
> Do you think I need to do a DEP on this ?
> If not, do you think it's ok if https://github.com/django/django/pull/7515 is
> changed to only do the readonly part, and we make another PR (and possibly
> ticket) for the "auto_refresh" part ?
>
> While it would be nice to have both landing at the same time on master, I
> feel there's nothing blocking if they land at separate time, even in
> separate versions if need be (the freeze on Django 1.11 is coming fast,
> IIRC).
>
> Are there usecases that I'm missing and that could not be expressed as a
> mix of readonly and auto_refresh ?
>
> Le lundi 7 novembre 2016 15:35:38 UTC+1, Aymeric Augustin a écrit :
>>
>> On 7 Nov 2016, at 13:44, Joachim Jablon  wrote:
>>
>> > My own personal opinion: 1. refesh by default, add an argument to
>> "model_instance.save()" to opt-out. 2. readonly
>>
>> I agree.
>>
>> --
>> Aymeric.
>>
>> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/django-developers/3db52e5b-54db-4736-845b-
> 58a22b6f015c%40googlegroups.com
> 
> .
>
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMwjO1FC1EAdJACQ8f3Hxy%2B13XPBh56KTfGMPeKyY2Ss_1zXqQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: New deferred loading implementation (Was: [Review Request] Added support for database delegated fields (like AutoField))

[Joachim Jablon]

We were having a discussion on this matter on the Django under the Hood 
Slack channel, and there's a design decision that I think I can't take by 
myself.

There are 2 related subject : the "readonly" part and the "auto_refresh" 
part. Readonly means that the database won't try to write, autorefresh 
means that when creating /updating an instance, the value will be fetched 
from the database automatically

There was a debate on whether the readonly behaviour should imply 
auto_refresh, and there are cases that are good candidates for auto_refresh 
without readonly (autofields, serial fields etc.).

What I'd suggest, if we can get an agreement on this, would be to define 
those 2 behaviours completely independently (possibly as 2 different PRs, 
each with their tests and docs and such).

Auto_refresh could then be used for autofields with the quirk that some SQL 
DBs, will provide last_insert_rowid() (sqlite) or LAST_INSERT_ID() (mysql), 
while other use RETURNING (PGSQL / Oracle) allowing to use a single query.

Readonly, on its side, would only be a simple independent feature and its 
behaviour would do what the title says, no more, no less.

Do you think I need to do a DEP on this ?
If not, do you think it's ok if https://github.com/django/django/pull/7515 is 
changed to only do the readonly part, and we make another PR (and possibly 
ticket) for the "auto_refresh" part ?

While it would be nice to have both landing at the same time on master, I 
feel there's nothing blocking if they land at separate time, even in 
separate versions if need be (the freeze on Django 1.11 is coming fast, 
IIRC).

Are there usecases that I'm missing and that could not be expressed as a 
mix of readonly and auto_refresh ?

Le lundi 7 novembre 2016 15:35:38 UTC+1, Aymeric Augustin a écrit :
>
> On 7 Nov 2016, at 13:44, Joachim Jablon > 
> wrote: 
>
> > My own personal opinion: 1. refesh by default, add an argument to 
> "model_instance.save()" to opt-out. 2. readonly 
>
> I agree. 
>
> -- 
> Aymeric. 
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3db52e5b-54db-4736-845b-58a22b6f015c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: New deferred loading implementation (Was: [Review Request] Added support for database delegated fields (like AutoField))

[Aymeric Augustin]

On 7 Nov 2016, at 13:44, Joachim Jablon  wrote:

> My own personal opinion: 1. refesh by default, add an argument to 
> "model_instance.save()" to opt-out. 2. readonly

I agree.

-- 
Aymeric.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/EF8CAC29-24FD-4FC1-AC18-ABBE98937846%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.

Re: New deferred loading implementation (Was: [Review Request] Added support for database delegated fields (like AutoField))

[Joachim Jablon]

So the PR followed here : https://github.com/django/django/pull/7515

The PR is based on the idea defined by Ben just above.

It raised 2 questions which I'd like to bring to your attention :

1. When inserting or updating a model instance that contains one or more 
readonly fields, should we fetch the readonly fields values from the 
database (instance.refresh_from_db(fields=readonly_fields))
Pro : 
- Consistent with AutoFields
- Easier to work with
- Can be done without an extra query for PGSQL using RETURNING (but it's 
out of scope for this ticket. That being said, it could be done within the 
1.11 scope)
Con :
- Intruduces an extra select query for models that have at least 1 readonly 
field.
- Can be easily done manually (especially if we provide a model instance 
such as ".refresh_readonly_fields()")
Alternative choice :
Give the choice to the user (in this case we still need to choose the 
default behaviour)

It's worth noting that the only affected models are those that include a 
"readonly" field, so that's not an extra select for everyone but only for 
the adopters of the new feature. It's also worth noting that the select is 
affecting a single element, using its pk, and targetting only the readonly 
fields.

2. For now this feature is named "readonly" and its default value is False. 
There was a suggestion to rather user the word "writable" and have its 
default value to True. I think both terms carry the same information.
Pro "readonly"
It's easier to name things in the code ("readonly fields" is a simpler 
concept than "non-writable fields" or "unwritable fields")
Pro "writable"
I'm not sure, but there were 3 :+1: on the GitHub comment so it's probably 
clearer

My own personal opinion: 1. refesh by default, add an argument to 
"model_instance.save()" to opt-out. 2. readonly

Le samedi 5 novembre 2016 16:17:31 UTC+1, Ben Cole a écrit :
>
> As discussed with many core team members (Simon Charette, Josh Smeaton, 
> Marc Tamlyn, Tim Graham) at DUTH 2016 sprints, myself and Joachim 
> Jablon have proposed a new, simpler solution to this. See 
> https://code.djangoproject.com/ticket/27446
>
> The proposal therefore is to add a `readonly` option to the base `Field` 
>> class that when `True` would strip these fields from being compiled to SQL 
>> during `INSERT`s and `UPDATE`s. This allows for a very simple change that 
>> covers all possible write queries that Django may perform (including 
>> bulk_*).
>> There exists a proof of concept 
>> https://github.com/novafloss/django-readonly-field
>
>
> PR to follow soon...
>
> On Friday, 12 February 2016 12:32:11 UTC, Anssi Kääriäinen wrote:
>>
>> On Thursday, February 11, 2016 at 2:41:44 PM UTC+2, Florian Apolloner 
>> wrote:
>>>
>>> Oh, I somewhat missread and though there would be a new DEFERRED 
>>> argument, the backwards issue is easy enough though:
>>>
>>>  * Unless I miss something, YourModel.__init__ is Model.__init__ if the 
>>> user didn't change it -> pass is DEFERRED
>>>  * If the user changed it check for model._meta.new_style_deferred and 
>>> continue accordingly
>>>  * Raise a warning if the __init__ is a custom one and 
>>> new_style_deffered is not set…
>>>
>>
>> If we are going to introduce a backwards compat system for this, then I 
>> think I want to get rid of calling Model.__init__ at all when loading from 
>> DB. We get faster model initialization, and conceptually loading from DB is 
>> like unpickling which (correctly) doesn't call __init__.
>>
>> However, based on the comments in the PR, I think we are going to just 
>> document the change to __init__ and skip deprecation.
>>
>>  - Anssi
>>  
>>
>>>
>>> On Thursday, February 11, 2016 at 1:38:44 PM UTC+1, Florian Apolloner 
>>> wrote:



 On Thursday, February 11, 2016 at 10:51:59 AM UTC+1, Anssi Kääriäinen 
 wrote:
>
> Before doing any further work on this we should decide if the 
> Model.__init__() problem is bad enough to stop this cleanup, and if so, 
> do 
> anybody have any ideas of how to avoid the problem?
>

 I do not think Model.__init__() is anywhere near public API, add it to 
 the release notes and be done with it, worst case add a try/except around 
 it…

 Cheers,
 Florian 

>>>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/2e8733b3-8336-4dad-909c-6a97711a9aa3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: New deferred loading implementation (Was: [Review Request] Added support for database delegated fields (like AutoField))

[Ben Cole]

As discussed with many core team members (Simon Charette, Josh Smeaton, 
Marc Tamlyn, Tim Graham) at DUTH 2016 sprints, myself and Joachim 
Jablon have proposed a new, simpler solution to this. 
See https://code.djangoproject.com/ticket/27446

The proposal therefore is to add a `readonly` option to the base `Field` 
> class that when `True` would strip these fields from being compiled to SQL 
> during `INSERT`s and `UPDATE`s. This allows for a very simple change that 
> covers all possible write queries that Django may perform (including 
> bulk_*).
> There exists a proof of concept 
> https://github.com/novafloss/django-readonly-field


PR to follow soon...

On Friday, 12 February 2016 12:32:11 UTC, Anssi Kääriäinen wrote:
>
> On Thursday, February 11, 2016 at 2:41:44 PM UTC+2, Florian Apolloner 
> wrote:
>>
>> Oh, I somewhat missread and though there would be a new DEFERRED 
>> argument, the backwards issue is easy enough though:
>>
>>  * Unless I miss something, YourModel.__init__ is Model.__init__ if the 
>> user didn't change it -> pass is DEFERRED
>>  * If the user changed it check for model._meta.new_style_deferred and 
>> continue accordingly
>>  * Raise a warning if the __init__ is a custom one and new_style_deffered 
>> is not set…
>>
>
> If we are going to introduce a backwards compat system for this, then I 
> think I want to get rid of calling Model.__init__ at all when loading from 
> DB. We get faster model initialization, and conceptually loading from DB is 
> like unpickling which (correctly) doesn't call __init__.
>
> However, based on the comments in the PR, I think we are going to just 
> document the change to __init__ and skip deprecation.
>
>  - Anssi
>  
>
>>
>> On Thursday, February 11, 2016 at 1:38:44 PM UTC+1, Florian Apolloner 
>> wrote:
>>>
>>>
>>>
>>> On Thursday, February 11, 2016 at 10:51:59 AM UTC+1, Anssi Kääriäinen 
>>> wrote:

 Before doing any further work on this we should decide if the 
 Model.__init__() problem is bad enough to stop this cleanup, and if so, do 
 anybody have any ideas of how to avoid the problem?

>>>
>>> I do not think Model.__init__() is anywhere near public API, add it to 
>>> the release notes and be done with it, worst case add a try/except around 
>>> it…
>>>
>>> Cheers,
>>> Florian 
>>>
>>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/d34db1f8-db9e-45ef-a383-3573986677b5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

New deferred loading implementation (Was: [Review Request] Added support for database delegated fields (like AutoField))

[Anssi Kääriäinen]

On Thursday, February 11, 2016 at 2:41:44 PM UTC+2, Florian Apolloner wrote:
>
> Oh, I somewhat missread and though there would be a new DEFERRED argument, 
> the backwards issue is easy enough though:
>
>  * Unless I miss something, YourModel.__init__ is Model.__init__ if the 
> user didn't change it -> pass is DEFERRED
>  * If the user changed it check for model._meta.new_style_deferred and 
> continue accordingly
>  * Raise a warning if the __init__ is a custom one and new_style_deffered 
> is not set…
>

If we are going to introduce a backwards compat system for this, then I 
think I want to get rid of calling Model.__init__ at all when loading from 
DB. We get faster model initialization, and conceptually loading from DB is 
like unpickling which (correctly) doesn't call __init__.

However, based on the comments in the PR, I think we are going to just 
document the change to __init__ and skip deprecation.

 - Anssi
 

>
> On Thursday, February 11, 2016 at 1:38:44 PM UTC+1, Florian Apolloner 
> wrote:
>>
>>
>>
>> On Thursday, February 11, 2016 at 10:51:59 AM UTC+1, Anssi Kääriäinen 
>> wrote:
>>>
>>> Before doing any further work on this we should decide if the 
>>> Model.__init__() problem is bad enough to stop this cleanup, and if so, do 
>>> anybody have any ideas of how to avoid the problem?
>>>
>>
>> I do not think Model.__init__() is anywhere near public API, add it to 
>> the release notes and be done with it, worst case add a try/except around 
>> it…
>>
>> Cheers,
>> Florian 
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/1190f1dd-dda9-445b-8a1c-396584553dd1%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Carl Meyer]

Hi Anssi,

On 02/11/2016 02:51 AM, Anssi Kääriäinen wrote:
> I created a proof-of-concept pull request for improved deferred fields
> loading. See https://github.com/django/django/pull/6118. In short, it is
> faster and cleaner (no more dynamically created subclasses for deferred
> loading!) than the previous implementation, but we have a couple of
> backwards incompatibilities:
> 1) |Model.__init__()| will be called with value DEFERRED for those
> fields which are deferred. Previously those fields were not given to
> |__init__()| at all.
> 2) The class level attribute Model._deferred must be removed (we
> can't have class level attribute as we don't have dynamic classes any more)
> 3) A potential name clash for fields when one has a class method and
> field with clashing names.
> 
> Of these the Model.__init__() case could be problematic for those users
> who have overridden __init__().
> 
> Before doing any further work on this we should decide if the
> Model.__init__() problem is bad enough to stop this cleanup, and if so,
> do anybody have any ideas of how to avoid the problem?

Overriding Model.__init__() is specifically called out in the
documentation as something to avoid, and there is no mention in the
documentation of what is passed to __init__() in the case of defer/only.
I think this is a fine change to just document in the release notes.

Carl

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/56BCDB8F.4080408%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Florian Apolloner]

On Thursday, February 11, 2016 at 1:41:44 PM UTC+1, Florian Apolloner wrote:
>
>  * Unless I miss something, YourModel.__init__ is Model.__init__ if the 
> user didn't change it -> pass is DEFERRED
>

Sadly this is not true, you can check if __init__ is in dict though…

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4eb340e5-dfbb-49c4-a597-e24559a66e5d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Florian Apolloner]

Oh, I somewhat missread and though there would be a new DEFERRED argument, 
the backwards issue is easy enough though:

 * Unless I miss something, YourModel.__init__ is Model.__init__ if the 
user didn't change it -> pass is DEFERRED
 * If the user changed it check for model._meta.new_style_deferred and 
continue accordingly
 * Raise a warning if the __init__ is a custom one and new_style_deffered 
is not set…

On Thursday, February 11, 2016 at 1:38:44 PM UTC+1, Florian Apolloner wrote:
>
>
>
> On Thursday, February 11, 2016 at 10:51:59 AM UTC+1, Anssi Kääriäinen 
> wrote:
>>
>> Before doing any further work on this we should decide if the 
>> Model.__init__() problem is bad enough to stop this cleanup, and if so, do 
>> anybody have any ideas of how to avoid the problem?
>>
>
> I do not think Model.__init__() is anywhere near public API, add it to the 
> release notes and be done with it, worst case add a try/except around it…
>
> Cheers,
> Florian 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/e5689091-2c2a-4902-b348-9f57d4375cd2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Florian Apolloner]

On Thursday, February 11, 2016 at 10:51:59 AM UTC+1, Anssi Kääriäinen wrote:
>
> Before doing any further work on this we should decide if the 
> Model.__init__() problem is bad enough to stop this cleanup, and if so, do 
> anybody have any ideas of how to avoid the problem?
>

I do not think Model.__init__() is anywhere near public API, add it to the 
release notes and be done with it, worst case add a try/except around it…

Cheers,
Florian 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/35f2d046-530d-483f-9842-4f1820f8deaf%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Anssi Kääriäinen]

On Wednesday, February 3, 2016 at 8:47:16 AM UTC+2, Anssi Kääriäinen wrote:

> For the update_fields change, I think we can do that completely 
> separately. The same goes for changing the way how deferred fields are 
> implemented.
>

I created a proof-of-concept pull request for improved deferred fields 
loading. See https://github.com/django/django/pull/6118. In short, it is 
faster and cleaner (no more dynamically created subclasses for deferred 
loading!) than the previous implementation, but we have a couple of 
backwards incompatibilities:
1) Model.__init__() will be called with value DEFERRED for those fields 
which are deferred. Previously those fields were not given to __init__() at 
all.
2) The class level attribute Model._deferred must be removed (we can't 
have class level attribute as we don't have dynamic classes any more)
3) A potential name clash for fields when one has a class method and 
field with clashing names.

Of these the Model.__init__() case could be problematic for those users who 
have overridden __init__().

Before doing any further work on this we should decide if the 
Model.__init__() problem is bad enough to stop this cleanup, and if so, do 
anybody have any ideas of how to avoid the problem?

 - Anssi

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/ced9d758-4cfe-4012-b7fc-37aaf221a83e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Podrigal, Aron]

Like it has been discussed a while ago [1] about adding *db_default*, we
should stick with something similar to that and support updates as well.

My 2 cents here.
I like the idea Anssi has proposed to delegate as much as possible using
expressions. So I would propose similar to what discussed in the other
thread using updated_at = DateTimeField(db_default=Expression,
db_update=Expression) (I don't like the param name *db_update* but you got
the idea).

Updating existing records. I would opt for leaving out fields which have a
db_update parameter, but still allow to override this behavior by
specifying those fields explicitly instance.save(update_fields=...). And
the same for MyModel.objects.filter().update(**fields) we should not
include so called *auto fields* in the update statement but we should never
leave out a field explicitly specified, (we already have problems how to
change pk values on existing records, and we don't want more behavior like
that).

We should refresh values for records on insert and update using the
RETURNING clause for those databases which support it. And defer fetching
the new value for other databases and for update only, since for insert we
already have to fetch the pk anyway, so we can fetch the other values as
well. An interesting idea like Anssi proposed would be to allow refreshing
values behavior be controlled via the Expression.

For queryset methods like bulk_create and update we can already control
deferring values for fields using *.defer()* and *.only()*. So if one would
really want to save the extra overhead fetching new values from db after
update, this can be done by MyModel.objects.get(pk=instance.pk).update().

I don't like Field(db_computed=True) it adds very minimal control over how
and what that does.

To sum up:

1)  fields should have flags how they are used in insert / update queries.

2)  controlling behavior should be done via Expressions

3) Field api should add 2 attributes *db_default* and *db_update* (perhaps
some better param name)

4) Do not limit overriding save behavior on a per query bases.


[1]
https://groups.google.com/forum/#!msg/django-developers/3mcro17Gb40/NPINCcyyBAAJ


On Feb 6, 2016 6:24 AM, "Owais Lone"  wrote:

> Shai and Ayeric, thank you so much for the feedback. The PR did indeed
> snowball into a much bigger one that I had initially planned. I agree with
> all points except,
>
> > - controlling this behavior in the query rather than in the field
> definition — this avoids the awkward “ignore what the field says” query
> parameter
>
> The main reason this PR exists is to let the fields decide if they should
> be updated/refreshed or not. If we don't define this behavior on the
> fields, then the feature will never work with 3rd party apps as developers
> will have to consciously remember to use some method on query API. For
> example, we could add a pguuid field to postgresql package that sets
> behaves like a normal uuid field but calculates the value using
> postgresql's uuid function. This would only work if the we define the
> preference on the field itself and the query API implicitly respects that.
>
> The minimum public API we would need to make this happen is to add an
> option to the Field class, something like,
>
> > id = models.IntegerField(db_computed=True)
>
> `db_computed` would make django do 2 things, it would implicitly ignore
> the field when updating and inserting and it would auto fetch the value
> from the DB after every insert/update on supported backends. That's it.
> Everything else was added to make this a bit flexible, like to make a field
> behave like this only or INSERT or on UPDATE but I think even having just
> one param that does it for both insert and update would be awesome!
>
> --
> Owais
>
> On Monday, February 1, 2016 at 2:03:26 PM UTC+5:30, Aymeric Augustin wrote:
>>
>> > On 31 janv. 2016, at 22:55, Shai Berger  wrote:
>> >
>> > Your message seems to be confusing the queryset API with the
>> model-instance
>> > API.
>>
>> Oops :-(
>>
>> Anyway, it seems that we agree on:
>>
>> - controlling this behavior in the query rather than in the field
>> definition — this avoids the awkward “ignore what the field says” query
>> parameter
>> - trying not to provide separate APIs for insert and update behavior
>> - improving the save(update_fields=…) API to support inserts as well as
>> updates
>>
>> --
>> Aymeric.
>>
>> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/2ac56d4e-9259-4fa6-985b-4311686662b6%40googlegroups.com
> 

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Owais Lone]

Shai and Ayeric, thank you so much for the feedback. The PR did indeed 
snowball into a much bigger one that I had initially planned. I agree with 
all points except,

> - controlling this behavior in the query rather than in the field 
definition — this avoids the awkward “ignore what the field says” query 
parameter 

The main reason this PR exists is to let the fields decide if they should 
be updated/refreshed or not. If we don't define this behavior on the 
fields, then the feature will never work with 3rd party apps as developers 
will have to consciously remember to use some method on query API. For 
example, we could add a pguuid field to postgresql package that sets 
behaves like a normal uuid field but calculates the value using 
postgresql's uuid function. This would only work if the we define the 
preference on the field itself and the query API implicitly respects that.

The minimum public API we would need to make this happen is to add an 
option to the Field class, something like,

> id = models.IntegerField(db_computed=True)

`db_computed` would make django do 2 things, it would implicitly ignore the 
field when updating and inserting and it would auto fetch the value from 
the DB after every insert/update on supported backends. That's it. 
Everything else was added to make this a bit flexible, like to make a field 
behave like this only or INSERT or on UPDATE but I think even having just 
one param that does it for both insert and update would be awesome!

--
Owais

On Monday, February 1, 2016 at 2:03:26 PM UTC+5:30, Aymeric Augustin wrote:
>
> > On 31 janv. 2016, at 22:55, Shai Berger  > wrote: 
> > 
> > Your message seems to be confusing the queryset API with the 
> model-instance 
> > API. 
>
> Oops :-( 
>
> Anyway, it seems that we agree on: 
>
> - controlling this behavior in the query rather than in the field 
> definition — this avoids the awkward “ignore what the field says” query 
> parameter 
> - trying not to provide separate APIs for insert and update behavior 
> - improving the save(update_fields=…) API to support inserts as well as 
> updates 
>
> -- 
> Aymeric. 
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/2ac56d4e-9259-4fa6-985b-4311686662b6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Anssi Kääriäinen]

The options for save() API seem to be:
  1. refresh_fields=[...] for save() - if set, then issue RETURNING for 
fields mentioned in the refresh_fields API.
  2. expressions: my_model.updated_at = TransactionNow(); my_model.save(), 
where TransactionNow is flagged to refresh the value from DB post-save.
  3. field flags like done in the PR.

Field flags are nice for things you always want to load from database (say, 
updated_at).

If the use cases we target are "generate value in db on insert" and 
"generate value in db on insert or update", and only those, then I think we 
can go with just a flag that tells Django to always skip the model's value 
on save(), and to reload from DB always. 

We are going to override the assigned value on save(), but I think it has 
to be this way. Consider:
  a_object.save()  # a_object.updated_at is set to '2016-02-03' as it is 
database delegated
  a_object.save()  # a_object.updated_at has a value, so we should error if 
we don't ignore the value
forcing users to set the value to None pre-save breaks the whole field flag 
feature. We could go directly with expressions in that case. We could also 
use the value in the update and require the DB trigger to ignore the value.

We don't have a separate flag for doing the refresh on insert only, but 
that isn't IMO such a big problem. We are going to add a minor overhead to 
updating by refreshing a field value from the DB when it hasn't changed, 
but making the API simpler is worth this IMO.

So, my opinion is that for the field flags API we need just one flag. A 
name like db_generated comes to mind. This gives a nice way to support the 
most common use case - that of calculating an inserted_at/updated_at field 
directly in DB.

After we have the field flag API, I think we should aim for expressions 
support. For expressions we should have enough flags for allowing full 
control over save() behavior. So, if the db_generated field flag isn't 
enough for some use case, you can go low-level and use expressions 
directly. We could perhaps even allow db_generated value to be an 
expression - if so, that expression would be used on save() and 
bulk_create().

For the update_fields change, I think we can do that completely separately. 
The same goes for changing the way how deferred fields are implemented.

 - Anssi

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/dedc791d-232d-4c93-9716-687963ca5272%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Anssi Kääriäinen]

On Monday, February 1, 2016 at 10:33:26 AM UTC+2, Aymeric Augustin wrote:
>
> > On 31 janv. 2016, at 22:55, Shai Berger  > wrote: 
> > 
> > Your message seems to be confusing the queryset API with the 
> model-instance 
> > API. 
>
> Oops :-( 
>
> Anyway, it seems that we agree on: 
>
> - controlling this behavior in the query rather than in the field 
> definition — this avoids the awkward “ignore what the field says” query 
> parameter 
> - trying not to provide separate APIs for insert and update behavior 
> - improving the save(update_fields=…) API to support inserts as well as 
> updates 
>

For the .save() method we could go with a new argument fields=[...]. The 
way it works is that only the mentioned fields are inserted or updated. If 
you want the current behavior of update_fields, set force_update=True.

The main reason why update_fields patch didn't ever try to target insert 
statements was that the feature didn't make much sense. We didn't have 
database defaults and we didn't have any other sort of expressions support 
for inserts. There was very limited use cases for skipping fields in 
insert. I'm not sure we have much of an use case now either (notably one 
can write DbDefaultExpression that compiles to sql, params = "default", [] 
already). What is the main use case for this feature?

I've been toying around with an idea that we add a non-data descriptor (one 
implementing just __get__) for all fields of an model, and use this for 
deferred loading. Currently we use dynamically created subclasses of the 
model which is a hack. The way this would work is that if an instance's 
__dict__ doesn't have any value for given field, then we interpret that 
field as deferred. This would also give a natural way to refresh a given 
field from DB. You would set a field "deferred" manually by "del 
instance.field", and after that instance.field would reload that field. The 
descriptor wouldn't cost much, as non-data descriptors don't add any 
measurable overhead when the attribute is set on the instance's __dict__. I 
think the feature would be backwards compatible (though without trying it 
is hard to be sure).

This feature would also allow skipping fields on insert: instance = 
MyModel(); del instance.some_field; instance.save() -> some_field is 
skipped as it is deferred; instance.some_field -> some_field is refreshed 
from DB. This would close the gap in the APIs.

I would like to see support for expressions that can tell Django the field 
should be skipped in update/insert and if the field should be refreshed 
from database after update/insert. This would give a nice low-level API for 
manipulating the behavior of save() and bulk_insert(). I don't see a need 
to mix this with the current pull request. The biggest problem for such 
expressions is that they don't make any sense when using a model. For 
example TransactionNow() isn't a nice value for last_update field in any 
other context than saving. Not sure this is a big enough problem to worry, 
but something to consider anyways.

Refreshable expressions would give a natural way for skipping fields on 
insert. Also, things such as database defaults would be easy to use with 
the ORM:

class DbDefault(Expression):
refresh_on_insert = True
refresh_on_update = True

def as_sql(self, compiler, connection):
return "default", []

my_instance = MyInstance(uuid=DbDefault())
print(my_instance.uuid) -> "DbDefault()"
my_instance.save()
print(my_instance.uuid) -> "f81d4fae-7dec-11d0-a765-00a0c91e6bf6"  (or 
whatever the DB happens to generate)

 - Anssi

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5310c9fc-d26a-469b-9a9c-dcdd437195b1%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Aymeric Augustin]

> On 31 janv. 2016, at 22:55, Shai Berger  wrote:
> 
> Your message seems to be confusing the queryset API with the model-instance 
> API.

Oops :-(

Anyway, it seems that we agree on:

- controlling this behavior in the query rather than in the field definition — 
this avoids the awkward “ignore what the field says” query parameter
- trying not to provide separate APIs for insert and update behavior
- improving the save(update_fields=…) API to support inserts as well as updates

-- 
Aymeric.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/FB10BF37-E6DD-48DD-AA40-ED0B18AE8DD8%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Shai Berger]

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

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Aymeric Augustin]

Hello Owais,

I had flagged this for review but it didn’t make it to the top of my list until 
today, sorry.

The current approach adds 7 new APIs that only target the use case exposed in 
the ticket, which I find rather narrow. Some of these APIs (ignore_delegated) 
only exist to work around the inflexibility of others. This doesn’t sound like 
the best possible design. I think there’s a more generally useful set of APIs 
that could make your use case possible, as well as others.

If I understood your PR correctly, you need five things, which I’m going to 
discuss below.


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’])

(Of course, the second and third need something like included_fields = 
all_fields - excluded_fields.)

Writing a custom manager to add the `defer()` clause automatically sounds 
preferable to adding a fourth way of doing the same thing, with a 
`delegated_on_update` keyword argument on fields.

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`?

Technically, the replacement for `.save(update_fields=fields)` would be 
`.only(*fields).save(force_update=True)` because `update_fields` forces an 
update. But I suppose most people won’t bother with the `force_update` argument 
if `.only()` does the right thing both on inserts and updates.


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. If they don’t have 
the same effect as on updates, I think we could consider it a bug and fix it.

Combined with a deprecation of `update_fields` for the reasons explained above, 
this would leave just `.defer()` and `.only()`, a consistent and versatile API 
for controlling which fields are included or excluded by `.save()`.
 

3) Return some values on updates
4) Return some values on inserts

I think a QuerySet-level method similar to `.only()` and `.defer()` would work 
here and would be more generally useful. Exposing this capability for databases 
that support it should be uncontroversial.

I haven’t given it too much thought. Others probably have.


5) A reasonably DRY way to specify all this

I think you should implement a custom default manager. This is the generic API 
Django gives to make this use case and many others possible.


To sum up, I’m suggesting to remove one API instead of adding seven, to achieve 
pretty much the same result :-) I realize you already put a lot of work into 
the approach I’m discouraging. Sorry about that. I hadn’t noticed this ticket 
before.

In any case, a long-winded discussion that results in a massive API is a clear 
sign that we mustn’t add one keyword argument for each possible behavior. 
Instead we must give tools for developers to describe the behavior they need in 
code.


Best regards,

-- 
Aymeric.


> On 6 janv. 2016, at 16:04, Owais Lone  wrote:
> 
> Hi, I would really appreciate if someone could review the API design (and 
> implementation) for a PR I created. The PR implements support for fields that 
> are assigned a value by the database (using a default value or trigger) 
> instead of Django. It also adds support for PostgreSQL’s RETURNING and 
> Oracle’s RETURNING INTO clauses.
> 
> This will allow us to have AutoField like behaviour on fields other than the 
> primary key or have primary key fields other than Integer and BigInteger 
> types. 
> 
> The PR also sets the stage for returning primary keys (and optionally other 
> fields) from the bulk_create operation on PostgreSQL and Oracle.
> 
> Pull request: https://github.com/django/django/pull/5904
> Django ticket: https://code.djangoproject.com/ticket/21454
> 
> Thanks
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Django developers  (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/CE5C6CD8-146E-4680-89A7-1351DF0CA76F%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.c

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Owais Lone]

Tim, I assumed the explanation and discussion on  the ticket would be 
enough. I'll write it up if I find some time next week.

Thanks

On Saturday, January 23, 2016 at 2:39:45 AM UTC+5:30, Tim Graham wrote:
>
> It would be helpful if you could sketch out the proposal a bit more -- 
> maybe write some documentation that explains and motivates the feature (and 
> put explanation in this thread). The pull request description has some 
> explanation but it doesn't seem suited toward someone unfamiliar with the 
> background of the ticket which could explain the lack of response here.
>
> On Wednesday, January 6, 2016 at 10:04:51 AM UTC-5, Owais Lone wrote:
>>
>> Hi, I would really appreciate if someone could review the API design (and 
>> implementation) for a PR I created. The PR implements support for fields 
>> that are assigned a value by the database (using a default value or 
>> trigger) instead of Django. It also adds support for PostgreSQL’s RETURNING 
>> and Oracle’s RETURNING INTO clauses. 
>>
>> This will allow us to have AutoField like behaviour on fields other than 
>> the primary key or have primary key fields other than Integer and 
>> BigInteger types. 
>>
>> The PR also sets the stage for returning primary keys (and optionally 
>> other fields) from the bulk_create operation on PostgreSQL and Oracle. 
>>
>> Pull request: https://github.com/django/django/pull/5904 
>> Django ticket: https://code.djangoproject.com/ticket/21454 
>>
>> Thanks
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3415d794-09ee-4adc-96e9-1f8a31e29f24%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Review Request] Added support for database delegated fields (like AutoField)

[Tim Graham]

It would be helpful if you could sketch out the proposal a bit more -- 
maybe write some documentation that explains and motivates the feature (and 
put explanation in this thread). The pull request description has some 
explanation but it doesn't seem suited toward someone unfamiliar with the 
background of the ticket which could explain the lack of response here.

On Wednesday, January 6, 2016 at 10:04:51 AM UTC-5, Owais Lone wrote:
>
> Hi, I would really appreciate if someone could review the API design (and 
> implementation) for a PR I created. The PR implements support for fields 
> that are assigned a value by the database (using a default value or 
> trigger) instead of Django. It also adds support for PostgreSQL’s RETURNING 
> and Oracle’s RETURNING INTO clauses. 
>
> This will allow us to have AutoField like behaviour on fields other than 
> the primary key or have primary key fields other than Integer and 
> BigInteger types. 
>
> The PR also sets the stage for returning primary keys (and optionally 
> other fields) from the bulk_create operation on PostgreSQL and Oracle. 
>
> Pull request: https://github.com/django/django/pull/5904 
> Django ticket: https://code.djangoproject.com/ticket/21454 
>
> Thanks

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/9612ab62-df34-438f-b475-1d96eb52ecb6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Review Request] Added support for database delegated fields (like AutoField)

[Owais Lone]

Hi, I would really appreciate if someone could review the API design (and 
implementation) for a PR I created. The PR implements support for fields that 
are assigned a value by the database (using a default value or trigger) instead 
of Django. It also adds support for PostgreSQL’s RETURNING and Oracle’s 
RETURNING INTO clauses.

This will allow us to have AutoField like behaviour on fields other than the 
primary key or have primary key fields other than Integer and BigInteger types. 

The PR also sets the stage for returning primary keys (and optionally other 
fields) from the bulk_create operation on PostgreSQL and Oracle.

Pull request: https://github.com/django/django/pull/5904
Django ticket: https://code.djangoproject.com/ticket/21454

Thanks

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CE5C6CD8-146E-4680-89A7-1351DF0CA76F%40gmail.com.
For more options, visit https://groups.google.com/d/optout.