Opened 2 years ago

Closed 12 months ago

Last modified 5 months ago

#34013 closed New feature (fixed)

Support ordering by annotation transforms (e.g JSONObject/ArrayAgg transforms).

Reported by: Eugene Morozov Owned by: Simon Charette
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: orm, json, ordering
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Please see the example:

queryset = Model.objects.all().annotate(
    json_field=functions.JSONObject(
        test_pk=models.Value(100) + models.F("pk"),
    ),
    test_pk_from_json_field=models.F("json_field__test_pk"),
)
print(queryset.order_by("json_field__test_pk"))  # AttributeError: 'JSONField' object has no attribute 'model'
print(queryset.order_by("test_pk_from_json_field"))  # Successful

Full traceback for ordering by json_field__test_pk:

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 256, in __repr__
    data = list(self[:REPR_OUTPUT_SIZE + 1])
  File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 280, in __iter__
    self._fetch_all()
  File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 1324, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 51, in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
  File "/usr/local/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 1162, in execute_sql
    sql, params = self.as_sql()
  File "/usr/local/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 513, in as_sql
    extra_select, order_by, group_by = self.pre_sql_setup()
  File "/usr/local/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 56, in pre_sql_setup
    order_by = self.get_order_by()
  File "/usr/local/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 356, in get_order_by
    order_by.extend(self.find_ordering_name(
  File "/usr/local/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 769, in find_ordering_name
    return [(OrderBy(transform_function(t, alias), descending=descending), False) for t in targets]
  File "/usr/local/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 769, in <listcomp>
    return [(OrderBy(transform_function(t, alias), descending=descending), False) for t in targets]
  File "/usr/local/lib/python3.10/site-packages/django/db/models/sql/query.py", line 1666, in transform
    wrapped = previous(field, alias)
  File "/usr/local/lib/python3.10/site-packages/django/db/models/sql/query.py", line 1641, in final_transformer
    return field.get_col(alias)
  File "/usr/local/lib/python3.10/site-packages/django/db/models/fields/__init__.py", line 397, in get_col
    if alias != self.model._meta.db_table or output_field != self:
AttributeError: 'JSONField' object has no attribute 'model'

It seems there is a lack of support JSONField in order_by queryset.

Change History (21)

comment:1 by Mariusz Felisiak, 2 years ago

Summary: order_by queryset does not work with annotated functions.JSONObject fieldsorder_by() crashes with annotated JSONObject fields.
Triage Stage: UnreviewedAccepted

Thanks for the report. Tentatively accepted for an investigation, I'm not sure if it's feasible.

comment:2 by Aman Pandey, 2 years ago

Owner: changed from nobody to Aman Pandey
Status: newassigned

comment:3 by Aman Pandey, 2 years ago

"It seems there is a lack of support JSONField in order_by queryset."
this is not true as JSONField based example queryset is working fine with order_by method

i guess it's more likely lack of support for lookup expression on annotated JSONObject

Last edited 2 years ago by Aman Pandey (previous) (diff)

comment:4 by Aman Pandey, 2 years ago

i think the issue is JSONObject returns a JSONField and since it's annotated JSONField doesn't get populated with the details of model it belongs to
since field generally are called in a model so they get populated with details of that model , here JSONField didn't get any details , including "self. model"
which causes an error .

so the main error occurs when order_by gets a lookup expression "" with double underscore , and by default it looks for an in relationship model's field which is populated by model's info but JSONField returned by JSONObject is not populated .thus throwing an error .

Version 2, edited 2 years ago by Aman Pandey (previous) (next) (diff)

comment:5 by Aman Pandey, 2 years ago

Resolution: fixed
Status: assignedclosed

i was trying to populate the Model and freshly created JSONField with each other's info like trying to populate JSONField.model with model info and
model._meta with JSONField info while the execution of annotate-> order_by-> add_ordering ->names_to_path (where lookup is processed ) but

that approach is very naive and making me fall deeper.

comment:6 by Aman Pandey, 2 years ago

Resolution: fixed
Status: closednew

in reply to:  description comment:7 by Rodolfo Valentín Becerra García, 2 years ago

Try this

from django.db.models.fields.json import KeyTextTransform
Model.objects.all().annotate(
    json_field=functions.JSONObject(
        test_pk=models.Value(100) + models.F("pk"),
    ),
    test_pk_from_json_field=KeyTextTransform(
        "test_pk", "json_field"
    )
)
Last edited 2 years ago by Rodolfo Valentín Becerra García (previous) (diff)

comment:8 by Saad Aleem, 20 months ago

Is this issue still valid? I'm trying to find a good first issue to create a PR for and did some digging here.

It looks like a model is attached to a Field object using the contribute_to_class method.

It seems that the contribute_to_class method is called when fields are added to the model class during its creation or when fields are added dynamically. In this case, we're not adding or modifying fields in the model class; we're only working with the queryset and computing new values based on existing fields. The queryset annotations do not modify the model class itself, so contribute_to_class is not called in this context.

comment:9 by Eugene Morozov, 20 months ago

I haven't tested this problem with the latest versions of Django 4.2 and postgres 3. Nonetheless, I don't believe that the issue is linked to the modifications made in Django 4.2.

So, you can try to run the code from the example and I think it will be still not working.

in reply to:  9 comment:10 by Saad Aleem, 20 months ago

Replying to Eugene Morozov:

I haven't tested this problem with the latest versions of Django 4.2 and postgres 3. Nonetheless, I don't believe that the issue is linked to the modifications made in Django 4.2.

So, you can try to run the code from the example and I think it will be still not working.

Thanks for the quick response.

I was able to reproduce it on the main branch but to fix it, a model would need to be attached to the JSONField inside the JSONObject method but it can't be done since annotations don't modify the model itself. So it seems like it might require a big architectural change for this to work.

Last edited 20 months ago by Saad Aleem (previous) (diff)

comment:11 by Mariusz Felisiak, 13 months ago

Summary: order_by() crashes with annotated JSONObject fields.order_by() crashes with annotated JSONObject/ArrayAgg fields.

#35025 was a duplicate for ArrayField returned by ArrayAgg.

comment:12 by Simon Charette, 13 months ago

Owner: changed from Aman Pandey to Simon Charette
Status: newassigned

comment:13 by Simon Charette, 13 months ago

Has patch: set

Feels like this is more of a new feature than a bug after all since adding support for JSONObject key ordering required adding generic support for ordering by annotation transforms.

comment:14 by Mariusz Felisiak, 13 months ago

Summary: order_by() crashes with annotated JSONObject/ArrayAgg fields.Support ordering by annotation transforms (e.g JSONObject/ArrayAgg transforms).
Type: BugNew feature

comment:15 by Mariusz Felisiak, 12 months ago

Needs documentation: set
Patch needs improvement: set

comment:16 by Simon Charette, 12 months ago

Needs documentation: unset
Patch needs improvement: unset

comment:17 by Mariusz Felisiak, 12 months ago

Triage Stage: AcceptedReady for checkin

comment:18 by Mariusz Felisiak <felisiak.mariusz@…>, 12 months ago

Resolution: fixed
Status: assignedclosed

In b0ad4119:

Fixed #34013 -- Added QuerySet.order_by() support for annotation transforms.

Thanks Eugene Morozov and Ben Nace for the reports.

comment:19 by GitHub <noreply@…>, 12 months ago

In eea4f92f:

Refs #34013 -- Registered instance lookups as documented in tests.

comment:20 by GitHub <noreply@…>, 5 months ago

In a16f13a:

Fixed #35643 -- Fixed a crash when ordering a QuerySet by a reference containing "".

Regression in b0ad41198b3e333f57351e3fce5a1fb47f23f376.

Refs #34013. The initial logic did not consider that annotation aliases
can include lookup or transform separators.

Thanks Gert Van Gool for the report and Mariusz Felisiak for the review.

comment:21 by Natalia <124304+nessita@…>, 5 months ago

In 55f5292:

[5.1.x] Fixed #35643 -- Fixed a crash when ordering a QuerySet by a reference containing "".

Regression in b0ad41198b3e333f57351e3fce5a1fb47f23f376.

Refs #34013. The initial logic did not consider that annotation aliases
can include lookup or transform separators.

Thanks Gert Van Gool for the report and Mariusz Felisiak for the review.
Backport of a16f13a8661297eda12c4177bb01fa2e5b5ccc56 from main.

Note: See TracTickets for help on using tickets.
Back to Top