Add JSONB_AGG to contrib.postgres

[Tomasz Nowak]

PostgreSQL 9.3 added many features for the JSON data type.
They include JSON_AGG aggregate, which could join its friends (ArrayAgg and StringAgg) in django/contrib/postgres/aggregates/general.py, please find draft implementation.
Function description: ​http://www.postgresql.org/docs/9.3/static/functions-aggregate.html

Other (non-aggregate) functions and expressions could go to a new file, maybe django/contrib/postgres/expressions.py? Please find attached JSON_BUILD_OBJECT implementation. It takes an array of column names and converts it to alternating key/value pairs for the function; the key is an extracted column name. It works for me, but I guess the column name (the key) should be somehow sanitized before appending to sql_expressions.

[Mads Jensen]

I tried to add this, and also made a test that aggregates a single field; it's adapted from the StringAgg-function. Would it make more sense to augment it so it's easier to aggregate several fields, or is this something that's better done in Python?

Also, JSONB is only available in 9.4+, so maybe some version checking would be a good idea? It might be slightly misleading that the JSON-datatype in django.contrib.postgres.fields is actually jsonb.

    class JsonAgg(Aggregate):
        function = 'JSONB_AGG'
        template = '%(function)s(%(expressions)s)'
        _output_field = JSONField()
    
        def convert_value(self, value, expression, connection, context):
            if not value:
                return json.dumps({})
            return value

[Simon Charette]

Would it make more sense to augment it so it's easier to aggregate several fields, or is this something that's better done in Python?

This can be done in a future iteration if required.

Also, JSONB is only available in 9.4+, so maybe some version checking would be a good idea?

You could throw an error by overriding as_sql() but I think a note in the documentation would be enough.

It might be slightly misleading that the JSON-datatype in django.contrib.postgres.fields is actually jsonb.

Agreed but lets not perpetuate this by naming the class JsonbAgg instead.

I find it a bit odd that we convert None to '' for StringConcat as it's not something we do for other aggregates (Count(NULL) is not converted to 0) and I'm not sure this is something we want to do for this aggregate as well. I can see a use case for having JsonbAgg return None and nothing prevents the users from wrapping the expression in a Coalesce if that's not what they expect.

[Tim Graham]

​PR with some test failures.

[Tim Graham]

Please uncheck "Patch needs improvement" after addressing the PR comments.

[Tim Graham <timograham@…>]

In 0a26f3c3:

Fixed #26327 -- Added JsonAgg to contrib.postgres.

Thanks Tim Graham for review.

[Tim Graham]

As described in #27478, the current implementation doesn't work on PostgreSQL 9.4 and below.

[Mads Jensen]

​PR Updated changes to remediate test failure on PostgreSQL 9.4. There is a misleading naming convention with JSONField and JsonAgg. The manual highlights that Jsonb is the preferred option.

[Simon Charette]

As mentioned above I think we should just rename the aggregate JSONBAgg (or whatever case variant we choose) and be done with it. It's unfortunate that we didn't name the contrib.postgres field JSONBField in the first place and have a database agnostic one named django.db.models.fields.JSONField now that other backends are shipping with support for such a field but we should make the distinction clear here.

We don't ship any field using the the PostgreSQL json datatype so it doesn't make much sense to ship an expression for the JSON_AGG SQL function IMHO. If we decide we still want to ship this tests should be added for it at least. That will require defining a JSONField relying on the json datatype.

[Tim Graham <timograham@…>]

In aa2cb4c:

Refs #26327 -- Renamed JsonAgg to JSONBAgg.

Thanks to Christian von Roques for the report.