Skip to content

GroupConcat missing filter parameter #947

Open

Description

@caramdache

Description

Native Django aggregates support a filter parameter that allow a subset of expressions to be returned. For example:

.annotate(count=Count("expr", filter=filter_expr))

which generates the following SQL:

COUNT(DISTINCT CASE WHEN <filter_expr> THEN <expr> ELSE NULL END) as count

However the filter parameter is ignored today by GroupConcat and I'm not quite sure how to add it to the source code.

Activity

  1. changed the title [-]group_concat missing filter parameter[/-] [+]GroupConcat missing `filter` parameter[/+] on Sep 13, 2022
  2. adamchainz commented on Sep 13, 2022

    @adamchainz
    Owner

    Can you check that GROUP_CONCAT supports this syntax, by showing some example queries?

    I might get around to adding this, but if you can try to open a PR that's a much better way of adding this feature. I don't use django-mysql very often these days, as most clients I work with use PostgreSQL.

  3. caramdache commented on Sep 14, 2022

    @caramdache
    Author

    I just created a PR with a fix, based on Django Count.

    Instead of creating the SQL manually, the Django Aggregate compiler is used instead. But that leaves out support for ORDER BY and SEPARATOR. Probably, this can be added by updating the template expression. But I did not do this for lack of time, understanding and filter was more important for me than the other two.

    Should someone figure how to do this, then the manual query creation code could probably go entirely. At the moment, it continues to be used when there is no filter expression.

  4. adamchainz commented on Sep 14, 2022

    @adamchainz
    Owner

    Instead of creating the SQL manually, the Django Aggregate compiler is used instead. But that leaves out support for ORDER BY and SEPARATOR. Probably, this can be added by updating the template expression. But I did not do this for lack of time, understanding and filter was more important for me than the other two.

    I've commented on the PR that I was confused by this. Now I see your comment that this was due to lack of time, I won't merge this in its current state. I'd like the filter pathway to be part of the existing compilation code.

  5. caramdache commented on Sep 14, 2022

    @caramdache
    Author

    I've added support for ORDER BY and SEPARATOR earlier today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions