[Django] #30711: Add HStoreF for F object like querying on HStoreField.

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

[Django] #30711: Add HStoreF for F object like querying on HStoreField.

Django
#30711: Add HStoreF for F object like querying on HStoreField.
-------------------------------------+-------------------------------------
               Reporter:  tiptop96   |          Owner:  (none)
                   Type:  New        |         Status:  new
  feature                            |
              Component:             |        Version:  2.2
  contrib.postgres                   |
               Severity:  Normal     |       Keywords:  HStoreField F
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 When using F objects for key lookups on HStoreFields it never digs down to
 get the keys but unexpectedly returns the entire object.

 Lets say we have this model:
 {{{
 from django.contrib.postgres.fields import HStoreField
 from django.db import models

 class Person(models.Model):
     name = models.CharField(max_length=256)
     attributes = HStoreField()
 }}}

 We then populate it.
 {{{
 Person.objects.create(name="James", attributes={"age": "30", "height":
 "190"})
 }}}

 Here comes the thing that bothers me. If we try to access either of these
 keys the entire attributes cell will be returned.
 {{{
 p = Person.objects.annotate(length=F("attributes__height"))
 p[0].height
 >>> {"age": "30", "height": "190"}
 }}}

 While the expected result would be an error or the value of the height
 field, it's not.

 I propose either making F objects support these fields or creating a new
 type to handle this, a workaround that I'm playing around with myself.

 {{{
 from django.contrib.postgres.fields.jsonb import KeyTransformFactory

 class HStoreF(F):
     def resolve_expression(self, query=None, allow_joins=True, reuse=None,
 summarize=False, for_save=False):
         rhs = super().resolve_expression(query, allow_joins, reuse,
 summarize, for_save)
         field_list = self.name.split("__")
         if field_list[-1] == rhs.target.name:
             raise LookupError(
             "HStoreF requires a key lookup in order to avoid unexpected
 behavior. "
             "Please append '__somekey' to '{}'."
             .format("__".join(field_list)))
         return KeyTransformFactory(field_list[-1])(rhs)

 }}}

 An issue with this is that updating models with it still wont work. As an
 error is raised in {{{Query.resolve_ref()}}} due to the fact that it
 interprets the {{{"__"}}} as an attempted join. This could be solved by
 using a different lookup separator for keys (maybe relevant for JSONField
 to?), but I was unable to successfully implement this.

 Similar issue with JSONField: https://code.djangoproject.com/ticket/29769

--
Ticket URL: <https://code.djangoproject.com/ticket/30711>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/051.fe579ab93d5dbcefd98faddf3e89c416%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30711: Add HStoreF for F object like querying on HStoreField.

Django
#30711: Add HStoreF for F object like querying on HStoreField.
----------------------------------+--------------------------------------
     Reporter:  Gustav Eiman      |                    Owner:  (none)
         Type:  New feature       |                   Status:  new
    Component:  contrib.postgres  |                  Version:  master
     Severity:  Normal            |               Resolution:
     Keywords:  HStoreField F     |             Triage Stage:  Unreviewed
    Has patch:  0                 |      Needs documentation:  0
  Needs tests:  0                 |  Patch needs improvement:  0
Easy pickings:  0                 |                    UI/UX:  0
----------------------------------+--------------------------------------
Changes (by felixxm):

 * version:  2.2 => master


Comment:

 Thanks for the report, I don't think that we should support this by adding
 a custom expression because currently you can always use
 `django.contrib.postgres.fields.hstore.KeyTransform`, e.g.
 {{{
 Person.objects.annotate(height=KeyTransform('height', 'attributes'))
 }}}
 I don't think that we need to add anything new. IMO, documentating
 `django.contrib.postgres.fields.hstore.KeyTransform` should be enough.

 It's more complicated for JSONField because nesting multiple
 `KeyTransform()` is not so handy.

--
Ticket URL: <https://code.djangoproject.com/ticket/30711#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.44e514fe552991bc351845564eaf08f1%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30711: Add HStoreF for F object like querying on HStoreField.

Django
In reply to this post by Django
#30711: Add HStoreF for F object like querying on HStoreField.
----------------------------------+--------------------------------------
     Reporter:  Gustav Eiman      |                    Owner:  (none)
         Type:  New feature       |                   Status:  new
    Component:  contrib.postgres  |                  Version:  master
     Severity:  Normal            |               Resolution:
     Keywords:  HStoreField F     |             Triage Stage:  Unreviewed
    Has patch:  0                 |      Needs documentation:  0
  Needs tests:  0                 |  Patch needs improvement:  0
Easy pickings:  0                 |                    UI/UX:  0
----------------------------------+--------------------------------------

Comment (by Gustav Eiman):

 Replying to [comment:1 felixxm]:
 > Thanks for the report, I don't think that we should support this by
 adding a custom expression because currently you can always use
 `django.contrib.postgres.fields.hstore.KeyTransform`, e.g.
 > {{{
 > Person.objects.annotate(height=KeyTransform('height', 'attributes'))
 > }}}
 > I don't think that we need to add anything new. IMO, documentating
 `django.contrib.postgres.fields.hstore.KeyTransform` should be enough.
 >
 > It's more complicated for JSONField because nesting multiple
 `KeyTransform()` is not so handy.

 Thank you! I agree, I had no idea this was available.

 Being a first time poster, what do I do now? Should I close this ticket?

--
Ticket URL: <https://code.djangoproject.com/ticket/30711#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.64f745b513ed2c6336342d5f5f2c9b9c%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30711: Document django.contrib.postgres.fields.hstore.KeyTransform. (was: Add HStoreF for F object like querying on HStoreField.)

Django
In reply to this post by Django
#30711: Document django.contrib.postgres.fields.hstore.KeyTransform.
--------------------------------------+------------------------------------
     Reporter:  Gustav Eiman          |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Documentation         |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:  HStoreField F         |             Triage Stage:  Accepted
    Has patch:  0                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  0
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------
Changes (by felixxm):

 * component:  contrib.postgres => Documentation
 * owner:  (none) => nobody
 * type:  New feature => Cleanup/optimization
 * stage:  Unreviewed => Accepted


Old description:

> When using F objects for key lookups on HStoreFields it never digs down
> to get the keys but unexpectedly returns the entire object.
>
> Lets say we have this model:
> {{{
> from django.contrib.postgres.fields import HStoreField
> from django.db import models
>
> class Person(models.Model):
>     name = models.CharField(max_length=256)
>     attributes = HStoreField()
> }}}
>
> We then populate it.
> {{{
> Person.objects.create(name="James", attributes={"age": "30", "height":
> "190"})
> }}}
>
> Here comes the thing that bothers me. If we try to access either of these
> keys the entire attributes cell will be returned.
> {{{
> p = Person.objects.annotate(length=F("attributes__height"))
> p[0].height
> >>> {"age": "30", "height": "190"}
> }}}
>
> While the expected result would be an error or the value of the height
> field, it's not.
>
> I propose either making F objects support these fields or creating a new
> type to handle this, a workaround that I'm playing around with myself.
>
> {{{
> from django.contrib.postgres.fields.jsonb import KeyTransformFactory
>
> class HStoreF(F):
>     def resolve_expression(self, query=None, allow_joins=True,
> reuse=None, summarize=False, for_save=False):
>         rhs = super().resolve_expression(query, allow_joins, reuse,
> summarize, for_save)
>         field_list = self.name.split("__")
>         if field_list[-1] == rhs.target.name:
>             raise LookupError(
>             "HStoreF requires a key lookup in order to avoid unexpected
> behavior. "
>             "Please append '__somekey' to '{}'."
>             .format("__".join(field_list)))
>         return KeyTransformFactory(field_list[-1])(rhs)
>
> }}}
>
> An issue with this is that updating models with it still wont work. As an
> error is raised in {{{Query.resolve_ref()}}} due to the fact that it
> interprets the {{{"__"}}} as an attempted join. This could be solved by
> using a different lookup separator for keys (maybe relevant for JSONField
> to?), but I was unable to successfully implement this.
>
> Similar issue with JSONField: https://code.djangoproject.com/ticket/29769
New description:

 Document `django.contrib.postgres.fields.hstore.KeyTransform` in the
 [https://docs.djangoproject.com/en/dev/ref/contrib/postgres/fields/#hstorefield
 HStoreField documentation] that can be used on the right hand side of a
 filter or an annotation.

--

Comment:

 I changed ticket description, thanks!

--
Ticket URL: <https://code.djangoproject.com/ticket/30711#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.c6d1ede2f09c230fca38b38c2c105102%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30711: Document django.contrib.postgres.fields.hstore.KeyTransform.

Django
In reply to this post by Django
#30711: Document django.contrib.postgres.fields.hstore.KeyTransform.
--------------------------------------+------------------------------------
     Reporter:  Gustav Eiman          |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Documentation         |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:  HStoreField F         |             Triage Stage:  Accepted
    Has patch:  0                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  0
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by tapaswenipathak):

 Hello folks: Can I take the ticket?

--
Ticket URL: <https://code.djangoproject.com/ticket/30711#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.e11b454ee4ff21501ce534a3add5ff34%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30711: Document django.contrib.postgres.fields.hstore.KeyTransform.

Django
In reply to this post by Django
#30711: Document django.contrib.postgres.fields.hstore.KeyTransform.
-------------------------------------+-------------------------------------
     Reporter:  Gustav Eiman         |                    Owner:
         Type:                       |  tapaswenipathak
  Cleanup/optimization               |                   Status:  assigned
    Component:  Documentation        |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  HStoreField F        |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Gustav Eiman):

 * owner:  nobody => tapaswenipathak
 * status:  new => assigned


Comment:

 Replying to [comment:4 tapaswenipathak]:
 > Hello folks: Can I take the ticket?
 That would be great! I'll assign it to you.

--
Ticket URL: <https://code.djangoproject.com/ticket/30711#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.8d9166ed07df26d29fb3b5709ccb6c4a%40djangoproject.com.