RFC: #30053 Allow for conditional QuerySet.update_or_create()

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFC: #30053 Allow for conditional QuerySet.update_or_create()

Joshua Cannon
Howdy folks!

At the suggestion of Simon Charette, I'd like to get feedback on how the community feels about the feature request #30053.
The gist of the problem is that sometimes the "update" part of "update_or_create" should be conditional. The proposed solution adds a backwards-compatible "update_condition" parameter to "update_or_create" which should be a unary callable that takes in the retrieved table instance and returns a boolean of whether the update should be performed.

Nasir Hussain has already started work on a possible solution on PR 10800.

I'd love to hear other people's thoughts.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/db8a538e-3219-41f1-9351-aba361bd89b8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: RFC: #30053 Allow for conditional QuerySet.update_or_create()

Simon Charette
Thanks for taking the time to post here Joshua!

The main reason why I asked to gather feedback from this list is that I'm not convinced
that the benefits outweighs the additional complexity this will add to the already quite
loaded update_or_create() signature handling and how the fact a callable predicate
is not something that is used anywhere else in Django AFAIK.

The fact this can be achieved with the same number of queries by doing a
save(update_fields) from the return value of a previous get_or_create with a
bit of boilerplate[0] pushes me to a -1 because I don't think it's a common enough
pattern to warrant the addition of a specialized option.

Cheers,
Simon

[0] https://code.djangoproject.com/ticket/30053#comment:13

Le vendredi 28 décembre 2018 11:15:33 UTC-5, Joshua Cannon a écrit :
Howdy folks!

At the suggestion of Simon Charette, I'd like to get feedback on how the community feels about the feature request <a href="https://code.djangoproject.com/ticket/30053" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F30053\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG1sCyiz6VW74v_SaK0Fm33iqN9yA&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F30053\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG1sCyiz6VW74v_SaK0Fm33iqN9yA&#39;;return true;">#30053.
The gist of the problem is that sometimes the "update" part of "update_or_create" should be conditional. The proposed solution adds a backwards-compatible "update_condition" parameter to "update_or_create" which should be a unary callable that takes in the retrieved table instance and returns a boolean of whether the update should be performed.

Nasir Hussain has already started work on a possible solution on PR <a href="https://github.com/django/django/pull/10800" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F10800\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG5hvDKgrBXiV7l1rX--DFsEa64gg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F10800\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG5hvDKgrBXiV7l1rX--DFsEa64gg&#39;;return true;">10800.

I'd love to hear other people's thoughts.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/82df287a-9e59-496b-956e-b0d4bd79760d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: RFC: #30053 Allow for conditional QuerySet.update_or_create()

Adam Johnson-2
I think the use case is probably quite niche, and I'd like to know some more concrete details than the current comment on the ticket "(E.g. only update if the value of a DateTimeField is less than some value)". It may be there's a way to achieve the same application-level outcome using a different design. If it is necessary, there are also some analogous use cases, like passing a 'create' condition predicate or passing conditions to get_or_create, that it seems it would make sense to implement at the same time but would complicate Django and the scope of the change further.

Also I agree with Simon that passing around predicate callables doesn't feel very Django-ey or pythonic.

On Fri, 28 Dec 2018 at 16:31, charettes <[hidden email]> wrote:
Thanks for taking the time to post here Joshua!

The main reason why I asked to gather feedback from this list is that I'm not convinced
that the benefits outweighs the additional complexity this will add to the already quite
loaded update_or_create() signature handling and how the fact a callable predicate
is not something that is used anywhere else in Django AFAIK.

The fact this can be achieved with the same number of queries by doing a
save(update_fields) from the return value of a previous get_or_create with a
bit of boilerplate[0] pushes me to a -1 because I don't think it's a common enough
pattern to warrant the addition of a specialized option.

Cheers,
Simon


Le vendredi 28 décembre 2018 11:15:33 UTC-5, Joshua Cannon a écrit :
Howdy folks!

At the suggestion of Simon Charette, I'd like to get feedback on how the community feels about the feature request #30053.
The gist of the problem is that sometimes the "update" part of "update_or_create" should be conditional. The proposed solution adds a backwards-compatible "update_condition" parameter to "update_or_create" which should be a unary callable that takes in the retrieved table instance and returns a boolean of whether the update should be performed.

Nasir Hussain has already started work on a possible solution on PR 10800.

I'd love to hear other people's thoughts.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/82df287a-9e59-496b-956e-b0d4bd79760d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
Adam

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAMyDDM1k7d1cHdDVROhB5GYrXdreJk%3DcSMYdeJbogB_C%3Dp5EYQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: RFC: #30053 Allow for conditional QuerySet.update_or_create()

Joshua Cannon
Personally, the snippet suggested in the ticket is sufficient for my needs (although I'm saddened by having to duplicate the setattr logic).
However, I filed the ticket after Googling "Django conditional update_or_create" and finding no appropriate solution. You might be right about the use case being niche, however others have ran into this too, with the proposed solutions being less-than-ideal.
(I won't link anything here, but searching stackoverflow for "Django conditional update_or_create" should lead you to the same question/answers I came across).

Perhaps the "solution" here isn't additional code, but rather a section in documentation that suggests a solution? It's extremely unfortunate that this is easy to get wrong in so many ways, but also isn't appropriate to add directly to Django.



On Friday, December 28, 2018 at 12:38:04 PM UTC-6, Adam Johnson wrote:
I think the use case is probably quite niche, and I'd like to know some more concrete details than the current comment on the ticket "(E.g. only update if the value of a DateTimeField is less than some value)". It may be there's a way to achieve the same application-level outcome using a different design. If it is necessary, there are also some analogous use cases, like passing a 'create' condition predicate or passing conditions to get_or_create, that it seems it would make sense to implement at the same time but would complicate Django and the scope of the change further.

Also I agree with Simon that passing around predicate callables doesn't feel very Django-ey or pythonic.

On Fri, 28 Dec 2018 at 16:31, charettes <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="nAQo5rFrDQAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">chare...@...> wrote:
Thanks for taking the time to post here Joshua!

The main reason why I asked to gather feedback from this list is that I'm not convinced
that the benefits outweighs the additional complexity this will add to the already quite
loaded update_or_create() signature handling and how the fact a callable predicate
is not something that is used anywhere else in Django AFAIK.

The fact this can be achieved with the same number of queries by doing a
save(update_fields) from the return value of a previous get_or_create with a
bit of boilerplate[0] pushes me to a -1 because I don't think it's a common enough
pattern to warrant the addition of a specialized option.

Cheers,
Simon

[0] <a href="https://code.djangoproject.com/ticket/30053#comment:13" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F30053%23comment%3A13\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEUmxdOJBYD7XcoT4ZNo6NSLf7Sdg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F30053%23comment%3A13\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEUmxdOJBYD7XcoT4ZNo6NSLf7Sdg&#39;;return true;">https://code.djangoproject.com/ticket/30053#comment:13

Le vendredi 28 décembre 2018 11:15:33 UTC-5, Joshua Cannon a écrit :
Howdy folks!

At the suggestion of Simon Charette, I'd like to get feedback on how the community feels about the feature request <a href="https://code.djangoproject.com/ticket/30053" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F30053\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG1sCyiz6VW74v_SaK0Fm33iqN9yA&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F30053\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG1sCyiz6VW74v_SaK0Fm33iqN9yA&#39;;return true;">#30053.
The gist of the problem is that sometimes the "update" part of "update_or_create" should be conditional. The proposed solution adds a backwards-compatible "update_condition" parameter to "update_or_create" which should be a unary callable that takes in the retrieved table instance and returns a boolean of whether the update should be performed.

Nasir Hussain has already started work on a possible solution on PR <a href="https://github.com/django/django/pull/10800" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F10800\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG5hvDKgrBXiV7l1rX--DFsEa64gg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F10800\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG5hvDKgrBXiV7l1rX--DFsEa64gg&#39;;return true;">10800.

I'd love to hear other people's thoughts.

--
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 <a href="javascript:" target="_blank" gdf-obfuscated-mailto="nAQo5rFrDQAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">django-develop...@googlegroups.com.
To post to this group, send email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="nAQo5rFrDQAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">django-d...@googlegroups.com.
Visit this group at <a href="https://groups.google.com/group/django-developers" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/group/django-developers&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/django-developers&#39;;return true;">https://groups.google.com/group/django-developers.
To view this discussion on the web visit <a href="https://groups.google.com/d/msgid/django-developers/82df287a-9e59-496b-956e-b0d4bd79760d%40googlegroups.com?utm_medium=email&amp;utm_source=footer" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/msgid/django-developers/82df287a-9e59-496b-956e-b0d4bd79760d%40googlegroups.com?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/msgid/django-developers/82df287a-9e59-496b-956e-b0d4bd79760d%40googlegroups.com?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;">https://groups.google.com/d/msgid/django-developers/82df287a-9e59-496b-956e-b0d4bd79760d%40googlegroups.com.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.


--
Adam

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/14f8cae0-0df1-4974-b80f-41e689e0d0d8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: RFC: #30053 Allow for conditional QuerySet.update_or_create()

Collin Anderson-2
Seems to me a documentation example or improvement could be helpful. 

On Mon, Dec 31, 2018 at 11:30 AM Joshua Cannon <[hidden email]> wrote:
Personally, the snippet suggested in the ticket is sufficient for my needs (although I'm saddened by having to duplicate the setattr logic).
However, I filed the ticket after Googling "Django conditional update_or_create" and finding no appropriate solution. You might be right about the use case being niche, however others have ran into this too, with the proposed solutions being less-than-ideal.
(I won't link anything here, but searching stackoverflow for "Django conditional update_or_create" should lead you to the same question/answers I came across).

Perhaps the "solution" here isn't additional code, but rather a section in documentation that suggests a solution? It's extremely unfortunate that this is easy to get wrong in so many ways, but also isn't appropriate to add directly to Django.



On Friday, December 28, 2018 at 12:38:04 PM UTC-6, Adam Johnson wrote:
I think the use case is probably quite niche, and I'd like to know some more concrete details than the current comment on the ticket "(E.g. only update if the value of a DateTimeField is less than some value)". It may be there's a way to achieve the same application-level outcome using a different design. If it is necessary, there are also some analogous use cases, like passing a 'create' condition predicate or passing conditions to get_or_create, that it seems it would make sense to implement at the same time but would complicate Django and the scope of the change further.

Also I agree with Simon that passing around predicate callables doesn't feel very Django-ey or pythonic.

On Fri, 28 Dec 2018 at 16:31, charettes <[hidden email]> wrote:
Thanks for taking the time to post here Joshua!

The main reason why I asked to gather feedback from this list is that I'm not convinced
that the benefits outweighs the additional complexity this will add to the already quite
loaded update_or_create() signature handling and how the fact a callable predicate
is not something that is used anywhere else in Django AFAIK.

The fact this can be achieved with the same number of queries by doing a
save(update_fields) from the return value of a previous get_or_create with a
bit of boilerplate[0] pushes me to a -1 because I don't think it's a common enough
pattern to warrant the addition of a specialized option.

Cheers,
Simon


Le vendredi 28 décembre 2018 11:15:33 UTC-5, Joshua Cannon a écrit :
Howdy folks!

At the suggestion of Simon Charette, I'd like to get feedback on how the community feels about the feature request #30053.
The gist of the problem is that sometimes the "update" part of "update_or_create" should be conditional. The proposed solution adds a backwards-compatible "update_condition" parameter to "update_or_create" which should be a unary callable that takes in the retrieved table instance and returns a boolean of whether the update should be performed.

Nasir Hussain has already started work on a possible solution on PR 10800.

I'd love to hear other people's thoughts.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/82df287a-9e59-496b-956e-b0d4bd79760d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
Adam

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/14f8cae0-0df1-4974-b80f-41e689e0d0d8%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 [hidden email].
To post to this group, send email to [hidden email].
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/CAFO84S7-g5EBxXZ5-SV1fRfc-3NidPnsXMK0-2fH%3DZFtbY6OmQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.