search

r0man / sqlingvo

A Clojure & ClojureScript DSL for SQL
Eclipse Public License 1.0
210 stars 23 forks source link

Support for ORDER BY in aggregate functions #52

Closed vincent-dm closed 8 years ago

vincent-dm commented 8 years ago

Hi,

Working with sqlingvo has been great, but I stumbled upon a situation that I think is not supported.

When using arrag_agg or json_agg, postgres allows you to add an ORDER BY clause before the closing parenthesis to make sure the aggregated data are in the right order. (MySQL uses the same notation in its group_concat function)

However, when I try this with sqlingvo, a comma gets added before the ORDER BY clause, resulting in a syntax error.

I'm pretty new to Clojure, so forgive me for not being able to propose a pull request, but based on issue #22 it seems like this might be fixed by creating some kind of exception for code within an arrag_agg or json_agg?

Would be great if this would be added at some point, but in any case I want to thank the maintainer(s) for all their time invested in building this library!

r0man commented 8 years ago

@vincent-dm Yes, you are right, this is missing at the moment. I added ORDER BY support for aggregate expressions over here: https://github.com/r0man/sqlingvo/pull/53 Let me know if this solves your problem, and I'll merge this.

Note, support for FILTER and WITHIN GROUP is still missing as described here: https://www.postgresql.org/docs/current/static/sql-expressions.html#SYNTAX-AGGREGATES

vincent-dm commented 8 years ago

Thanks for the lightning-quick response, @r0man

I'd like to test it immediately, but I've never loaded a specific git-commit in my project.clj before. Should I use something like https://github.com/tobyhede/lein-git-deps or are there a better way?

r0man commented 8 years ago

@vincent-dm I usually clone a project project I want to test, switch to that branch, install the jar locally and then use the version from my local maven cache.

However, I just pushed a snapshot to Clojars, so just try 0.8.14-SNAPSHOT in your project.clj

r0man commented 8 years ago

I haven't used lein-git-deps, maybe that's also a fine option.

vincent-dm commented 8 years ago

@r0man: I tested it, and it works like a charm.

Thank you. I couldn't hope for better support than this.

r0man commented 8 years ago

@vincent-dm Thanks for the confirmation, I released this as 0.8.14.