﻿id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
35339	Ordering and filtering a Postgres ArrayAgg with parameters inverts SQL param order	Chris M	Chris M	"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."	Bug	closed	Database layer (models, ORM)	dev	Normal	fixed	postgres arrayagg ordering	Mariusz Felisiak Simon Charette	Ready for checkin	1	0	0	0	0	0
