Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  postgres arrayagg| Triage Stage:  Ready for
  ordering   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Natalia Bidart):

 #35613 was a duplicate.
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/01070190c6a0cdd8-3a607961-3224-4fe4-b02b-4afb4a252838-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  postgres arrayagg| Triage Stage:  Ready for
  ordering   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Chris M):

 Thanks, Simon. You are right, I mentioned `ArrayAgg` but I should have
 been referencing the `StringAgg` function. I appreciate the input and gut-
 check commit. The commit seems to answer a many of the early questions I
 had cropping up internally as I started playing around with the concept
 around the best way to structure the code and handle some of the feature
 checks.

 I take a crack at creating a new ticket based on your input here and start
 working on it.
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018f67ba1160-12d000ce-1c36-477a-9cf7-1ca778ec452c-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  postgres arrayagg| Triage Stage:  Ready for
  ordering   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Simon Charette):

 Hello Chris, glad to hear you're interested in moving this forward! We
 should definitely create a new ticket for that.

 I'd suggest targeting `StringAgg(Aggregate)` and default to using
 `STRING_AGG` (Postgres, SQLite), `GROUP_CONCAT` as fallback on MySQL, and
 `LIST_AGG WITHIN GROUP (ORDER BY)` on Oracle.

 As for whether `StringAgg` should live in `models.aggregate` (or only in
 tests at first) I think it could even if that means documenting it.
 Another potential candidate that could be added is `JSONArrayAgg` but I
 think we should focus on a single one for this first ticket.

 A few things I believe we should focus on along the way

 1. We should make the argument `Aggregate(order_by)` instead of `ordering`
 to be consistent with `Window`
 2. Just like we do with `allow_distinct` we should use a `allow_order_by`
 attribute for aggregates that support this feature
 3. We should make `OrderableAggMixin` a shim that sets
 `allow_order_by=True`, redirect `__init__(ordering)` to `order_by`, and
 emit a deprecation warning when doing so to allow a proper transition for
 `contrib.postgres.ArrayAgg` and friends
 4. We should reuse `OrderByList` as much as possible

 If that can be of any help
 [https://github.com/charettes/django/commits/ticket-35339-generic-order-
 by-aggregate/ I hacked a bit on it a few days ago to make sure I didn't
 send through a rabbit hole] by requesting these
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018f57baf96f-2615aa72-a530-471b-9133-56167532ae2f-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  postgres arrayagg| Triage Stage:  Ready for
  ordering   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Chris M):

 Simon, I am interested in working on the second phase of merging in
 `OrderableAggMixin` . I started playing around with this code, and I think
 it makes sense. My initial question is around how moving this up looks
 with regards to things like `ArrayAgg`. That function directly exists in
 other databases, like Sqlite, so would we want to put it into the core
 aggregate module instead of the postgres module? To build on that, MySQL
 implements basically the same behavior as `group_concat`, which Sqlite
 also supports, so is there some nuance to how that should be handled as
 well?

 Also, should we create a new ticket for this second phase of the work?
 What would that look like?
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018f560b8142-1385fd95-f031-4b56-a934-fb85fba41e25-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  postgres arrayagg| Triage Stage:  Ready for
  ordering   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by GitHub ):

 In [changeset:"8c257cecffc314179a5375ab8a16ca0ff3b6fb16" 8c257cec]:
 {{{#!CommitTicketReference repository=""
 revision="8c257cecffc314179a5375ab8a16ca0ff3b6fb16"
 Refs #35339 -- Fixed source expressions in GeoAggregate on Oracle.

 Regression in 42b567ab4c5bfb1bbd3e629b1079271c5ae44ea0.
 }}}
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018f1d32160b-c0993497-16f7-446a-bb49-7cf50c6b93ed-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  postgres arrayagg| Triage Stage:  Ready for
  ordering   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by nessita <124304+nessita@…>):

 In [changeset:"42b567ab4c5bfb1bbd3e629b1079271c5ae44ea0" 42b567ab]:
 {{{#!CommitTicketReference repository=""
 revision="42b567ab4c5bfb1bbd3e629b1079271c5ae44ea0"
 Refs #35339 -- Updated Aggregate class to return consistent source
 expressions.

 Refactored the filter and order_by expressions in the Aggregate class to
 return a list of Expression (or None) values, ensuring that the list
 item is always available and represents the filter expression.
 For the PostgreSQL OrderableAggMixin, the returned list will always
 include the filter and the order_by value as the last two elements.

 Lastly, emtpy Q objects passed directly into aggregate objects using
 Aggregate.filter in admin facets are filtered out when resolving the
 expression to avoid errors in get_refs().

 Thanks Simon Charette for the review.
 }}}
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018f16fcac9f-5ebc2307-02c2-47a0-a405-e1078b14666b-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  postgres arrayagg| Triage Stage:  Ready for
  ordering   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by nessita <124304+nessita@…>):

 * resolution:   => fixed
 * status:  assigned => closed

Comment:

 In [changeset:"c8df2f994130d74ec35d32a36e30aad7d6ea8e3a" c8df2f99]:
 {{{#!CommitTicketReference repository=""
 revision="c8df2f994130d74ec35d32a36e30aad7d6ea8e3a"
 Fixed #35339 -- Fixed PostgreSQL aggregate's filter and order_by params
 order.

 Updated OrderableAggMixin.as_sql() to separate the order_by parameters
 from the filter parameters. Previously, the parameters and SQL were
 calculated by the Aggregate parent class, resulting in a mixture of
 order_by and filter parameters.

 Thanks Simon Charette for the review.
 }}}
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018f16fcacca-fd8ce3ce-b3cc-4883-ab7c-f856a30243cf-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  postgres arrayagg| Triage Stage:  Ready for
  ordering   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Natalia Bidart):

 * stage:  Accepted => Ready for checkin

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018f16fc1d1c-c5587c33-01dc-4ea0-8254-12fe3215a9e7-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  postgres arrayagg| Triage Stage:  Accepted
  ordering   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * needs_better_patch:  1 => 0

Comment:

 Patch is looking good to me albeit some commit message massaging that
 mergers should be able to handle.
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018eb450118d-e2d8d329-51b8-45ce-b071-59a0e7a41a6d-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  postgres arrayagg| Triage Stage:  Accepted
  ordering   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * needs_better_patch:  0 => 1

Comment:

 Left some comments on the PR for some subtle adjustments to get the tests
 passing but it's looking promising to me.
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ea735ba9f-60fe4ea2-9a35-4198-b519-f025d44b8bba-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  postgres arrayagg| Triage Stage:  Accepted
  ordering   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Chris M):

 * has_patch:  0 => 1

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ea6286266-3c2ca880-b354-4bd7-a998-041c2f54a74e-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  postgres arrayagg| Triage Stage:  Accepted
  ordering   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Simon Charette):

 Sounds good! I suggest you start by making both `Aggregate` and
 `OrderableAggMixin` unconditionally return `None` in their
 `source_expressions` getters and setters when they respectively don't have
 any `filter` and `ordering`.

 This should address the immediate issue reported in this ticket and from
 there, if you're feeling confident, merging `OrderableAggMixin` and
 dealing with its deprecation could be tackled in a follow up ticket.
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018e9b00cb6b-a46d3178-9531-4620-927b-253d587822e6-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  Chris M
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  postgres arrayagg| Triage Stage:  Accepted
  ordering   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Chris M):

 * owner:  nobody => Chris M
 * status:  new => assigned

Comment:

 I think I see what you are going for, Simon. I'll give that a go and send
 up a pull request once I have something to share.
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018e9a26ab53-b165b482-c077-417e-8edb-9c5b3da11b69-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  postgres arrayagg| Triage Stage:  Accepted
  ordering   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Simon Charette):

 Thank you for the detailed report Chris and for the regression test
 Natalia this is very useful!

 Since this a long standing issue and we thus don't have to deal with a
 backport I would suggest we bite the bullet and refactor things up to have
 `OrderableAggMixin` merged in `Aggregate` and have a flag similar to
 `allow_distinct`.

 The crux of the issue here is that we try be clever and have the
 `get_source_expressions` / `set_source_expressions` signature change
 depending on whether or not a `filter` and and `order_by` are assigned
 (the `len` of expressions will change) so we can satisfy
 `list[Expression]`. We tried doing that for a while with `Window` but it
 became clear that using `None` as placeholders for unspecified expressions
 was much easier to reason about pretty much everything expects
 `get_expressions` to be `list[Expression | None]`. Doing so avoids the
 conditional shenanigans that are causing out of order due to
 `expressions.pop` and such.

 Another big plus to having all the logic lives in `Aggregate` is that it
 makes implementing things like `STRING_AGG` (AKA `GROUP_CONCAT`) and
 [https://sqlite.org/releaselog/3_44_0.html other expressions that gain
 support for ordering overnight much easier] and tested in a generic way.

 If that's something you're interesting in working on Chris I'd be happy to
 provide you reviewing support.
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018e91ed5543-a80dbe46-c2a4-4e59-a70f-b646d53ee013-00%40eu-central-1.amazonses.com.

Re: [Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
 Reporter:  Chris M  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  postgres arrayagg| Triage Stage:  Accepted
  ordering   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Natalia Bidart):

 * cc: Mariusz Felisiak, Simon Charette (added)
 * keywords:   => postgres arrayagg ordering
 * stage:  Unreviewed => Accepted
 * type:  Uncategorized => Bug
 * version:  5.0 => dev

Comment:

 Hello Chris, thank you for your detailed report.

 I can confirm that the provided test fails in current `main` as shown
 below. Adding Simon and Mariusz as cc to see if they can provide advice on
 the

 {{{#!diff
 diff --git a/tests/postgres_tests/test_aggregates.py
 b/tests/postgres_tests/test_aggregates.py
 index 386c55da25..5ab27752d1 100644
 --- a/tests/postgres_tests/test_aggregates.py
 +++ b/tests/postgres_tests/test_aggregates.py
 @@ -12,7 +12,7 @@ from django.db.models import (
  Window,
  )
  from django.db.models.fields.json import KeyTextTransform, KeyTransform
 -from django.db.models.functions import Cast, Concat, Substr
 +from django.db.models.functions import Cast, Concat, LPad, Substr
  from django.test import skipUnlessDBFeature
  from django.test.utils import Approximate
  from django.utils import timezone
 @@ -188,6 +188,16 @@ class TestGeneralAggregate(PostgreSQLTestCase):
  )
  self.assertEqual(values, {"arrayagg": expected_output})

 +def test_array_agg_filter_and_ordering_params(self):
 +values = AggregateTestModel.objects.aggregate(
 +arrayagg=ArrayAgg(
 +"char_field",
 +filter=Q(json_field__has_key="lang"),
 +ordering=LPad(Cast("integer_field", CharField()), 2,
 Value("0")),
 +)
 +)
 +self.assertEqual(values, {"arrayagg": ["Foo2", "Foo4"]})
 +
  def test_array_agg_integerfield(self):
  values = AggregateTestModel.objects.aggregate(
  arrayagg=ArrayAgg("integer_field")
 }}}

 Full trace with `--debug-sql`:
 {{{
 ==
 ERROR: test_array_agg_filter_and_ordering_params
 
(postgres_tests.test_aggregates.TestGeneralAggregate.test_array_agg_filter_and_ordering_params)
 --
 Traceback (most recent call last):
   File "/home/nessita/fellowship/django/django/db/backends/utils.py", line
 105, in _execute
 return self.cursor.execute(sql, params)

   File "/home/nessita/.virtualenvs/djangodev/lib/python3.11/site-
 packages/psycopg/cursor.py", line 732, in execute
 raise ex.with_traceback(None)
 psycopg.errors.UndefinedFunction: function lpad(character varying,
 unknown, integer) does not exist
 LINE 1: ...s_tests_aggregatetestmodel"."char_field" ORDER BY LPAD(("pos...
  ^
 HINT:  No function matches the given name and argument types. You might
 need to add explicit type casts.

 The above exception was the direct cause of the following exception:

 Traceback (most recent call last):
 ...
   File "/home/nessita/.virtualenvs/djangodev/lib/python3.11/site-
 packages/psycopg/cursor.py", line 732, in execute
 raise ex.with_traceback(None)
 django.db.utils.ProgrammingError: function lpad(character varying,
 unknown, integer) does not exist
 LINE 1: ...s_tests_aggregatetestmodel"."char_field" ORDER BY LPAD(("pos...
  ^
 HINT:  No function matches the given name and argument types. You might
 need to add explicit type casts.

 --
 (0.000)
 SELECT ARRAY_AGG("postgres_tests_aggregatetestmodel"."char_field"
  ORDER BY
 LPAD(("postgres_tests_aggregatetestmodel"."integer_field")::varchar,
 'lang', 2)) FILTER (
 WHERE "postgres_tests_aggregatetestmodel"."json_field" ? '0') AS
 "arrayagg"
 FROM "postgres_tests_aggregatetestmodel";

 args=('lang',
   Int4(2),
   '0');

 ALIAS=DEFAULT
 }}}
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this messa

[Django] #35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order

[Django]

#35339: Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL
param order
-+-
   Reporter:  Chris M|  Owner:  nobody
   Type: | Status:  new
  Uncategorized  |
  Component:  Database   |Version:  5.0
  layer (models, ORM)|
   Severity:  Normal |   Keywords:
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  0
  UI/UX:  0  |
-+-
 When trying to build an ArrayAgg annotation that has a `filter` with
 parameters and a `ordering` with parameters, the SQL that is built will
 invert the parameters, putting the filter parameters before the ordering
 parameters. I was not able to find an existing ticket for this issue.

 I have verified this against the current dev branch as well as version
 3.2.

 I was able to reproduce this in a test in the test_aggregates.py file.

 {{{#!python
 def test_array_agg_filter_and_ordering_params(self):
 values = AggregateTestModel.objects.aggregate(
 arrayagg=ArrayAgg(
 "char_field",
 filter=Q(json_field__has_key="lang"),
 ordering=LPad(Cast("integer_field", CharField()), 2,
 Value("0")),
 )
 )
 self.assertEqual(values, {"arrayagg": ["Foo2", "Foo4"]})
 }}}

 The resulting error is something like

 {{{
 Traceback (most recent call last):
   File "/Users/camuthig/projects/oss/django/django/db/backends/utils.py",
 line 105, in _execute
 return self.cursor.execute(sql, params)
 psycopg2.errors.UndefinedFunction: function lpad(character varying,
 integer, integer) does not exist
 LINE 1: ...s_tests_aggregatetestmodel"."char_field" ORDER BY LPAD(("pos...
  ^
 HINT:  No function matches the given name and argument types. You might
 need to add explicit type casts.
 }}}

 The issue is that the result of the `OrderableAggMixin.as_sql` function is

 SQL:
 {{{
 ARRAY_AGG("postgres_tests_aggregatetestmodel"."char_field" ORDER BY
 LPAD(("postgres_tests_aggregatetestmodel"."integer_field")::varchar, %s,
 %s)) FILTER (WHERE "postgres_tests_aggregatetestmodel"."json_field" ? %s)
 }}}

 Parameters
 {{{
 [ "lang", 2, "0"]
 }}}

 So we are trying to use "lang" and 2 as the values for the ordering
 function, and "0" as the parameter for the filtering function. This is
 made a bit more confusing if the expression you are aggregating also has a
 parameter, because that should be before the ordering parameters. It
 should be

 1. Expression parameters
 2. Ordering parameters
 3. Filtering parameters

 This happens because both the expression and filtering parameters come
 from the standard Aggregate parent class, and are then put in front of the
 ordering parameters in the Postgres-specific orderable mixin.

 I have been able to resolve this issue locally by altering the
 `OrderableAggMixin.as_sql` function to retrieve the parameters from the
 parent and then split them manually.


 {{{#!python
 class OrderableAggMixin:
 # ... other functions

 def as_sql(self, compiler, connection):
 if self.order_by is not None:
 order_by_sql, order_by_params =
 compiler.compile(self.order_by)
 else:
 order_by_sql, order_by_params = "", ()

 sql, expression_params = super().as_sql(compiler, connection,
 ordering=order_by_sql)

 filter_params = ()
 if self.filter:
 try:
 _, filter_params = self.filter.as_sql(compiler,
 connection)
 expression_params =
 expression_params[:-len(filter_params)]
 except FullResultSet:
 pass

 return sql, (*expression_params, *order_by_params, *filter_params)
 }}}


 This solution technically works, but it feels a bit clunky, so I am open
 to suggestions on how to improve it. I can also create a pull request with
 the change if you would like to see it, though my changes are all captured
 here already.
-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018e855e5e26-a8034f57-cc14-4365-92a7-186c14ea8ddb-00%40eu-central-1.amazonses.com.