Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syntax error for simple grouping in Postgres or non-optimal sql #630

Open
cvogt opened this issue Jan 29, 2014 · 7 comments
Open

Syntax error for simple grouping in Postgres or non-optimal sql #630

cvogt opened this issue Jan 29, 2014 · 7 comments

Comments

@cvogt
Copy link
Member

cvogt commented Jan 29, 2014

From http://stackoverflow.com/questions/21414449/aggregate-multiple-columns-without-groupby-in-slick-2-0:

I would like to perform an aggregation with Slick that executes SQL like the following:

SELECT MIN(a), MAX(a) FROM table_a;

where table_a has an INT column a

In Slick given the table definition:

class A(tag: Tag) extends Table[Int](tag, "table_a") { 
  def a = column[Int]("a")
  def * = a 
}
val A = TableQuery[A]
val as = A.map(_.a)

It seems like I have 2 options:

  1. Write something like: Query(as.min, as.max)
  2. Write something like:
as
  .groupBy(_ => 1)
  .map { case (_, as) => (as.map(identity).min, as.map(identity).max) }

However, the generated sql is not good in either case. In 1, there are two separate sub-selects generated, which is like writing two separate queries. In 2, the following is generated:

select min(x2."a"), max(x2."a") from "table_a" x2 group by 1

However, this syntax is not correct for Postgres (it groups by the first column value, which is invalid in this case). Indeed AFAIK it is not possible to group by a constant value in Postgres, except by omitting the group by clause.

Is there a way to cause Slick to emit a single query with both aggregates without the GROUP BY?

@cvogt
Copy link
Member Author

cvogt commented Jan 29, 2014

Someone commented:
I use the following trick in SQL Server, and it seems to work in Postgres:

select min(x2."a"), max(x2."a")
from "table_a" x2
group by (case when x2.a = x2.a then 1 else 1 end);

The use of the variable in the group by expression tricks the compiler into thinking that there could be more than one group.

@szeiger
Copy link
Member

szeiger commented Jan 29, 2014

Doesn't #624 already take care of that? The GROUP BY isn't necessary in the first place in this example. You're aggregating over a whole view. #624 should remove the GROUP BY.

@szeiger szeiger added this to the 2.1 milestone Feb 10, 2014
@szeiger
Copy link
Member

szeiger commented Feb 10, 2014

Setting to improvement / 2.1 because the groupBy workaround should work in 2.0.1 but a more direct way to express this query would be preferable.

@szeiger szeiger modified the milestones: 2.2.0, 2.1.0 Jun 30, 2014
@szeiger
Copy link
Member

szeiger commented Jun 30, 2014

This can be expressed directly in Slick 2.1 but the generated code contains a duplicate subquery. Slick does not support unification of subqueries at the moment.

@szeiger
Copy link
Member

szeiger commented Jun 30, 2014

FTR, the code should look something like this:

val r5 = (ts.map(_.a).max, ts.map(_.a).min).shaped.run

This works in 2.1 but generates an unnecessarily complicated statement.

@szeiger szeiger modified the milestones: Future, 2.2.0 Jun 30, 2014
@mboogerd
Copy link

Out of curiosity, what is the status of this issue (w.r.t. Slick 3.X?). I have found myself wanting to express a similarly simple statement:

SELECT COUNT(T1.x), COUNT(T2.y)
FROM table1 AS T1
LEFT JOIN table2 AS T2
  ON T1.x = T2.y

I managed to get it working with groupBy, but couldn't figure out how to do it without. I found another related issue on SO about this (http://stackoverflow.com/questions/29278685/how-to-make-multiple-aggregations-with-slick-without-group-by), but no answers.

Is this aggregation expressible in Slick 3 without groupBy and have I just missed how? Or is it unresolved for both Slick 2 and 3?

If it isn't resolved, am I correct in assuming a native query would be the way to go if efficiency matters?

@szeiger
Copy link
Member

szeiger commented Sep 22, 2015

No change in 3.0 or 3.1. For example, AggregateTest.testAgregates produces the following for the last query:

   ┃           ┇ select s78.s70, s84.s72, s86.s74, s87.s76
┃   ┃           ┇ from (
┃   ┃           ┇   select count(1) as s70
┃   ┃           ┇   from "t2"
┃   ┃           ┇   where "a" = 1
┃   ┃           ┇ ) s78, (
┃   ┃           ┇   select sum("a") as s72
┃   ┃           ┇   from "t2"
┃   ┃           ┇   where "a" = 1
┃   ┃           ┇ ) s84, (
┃   ┃           ┇   select sum("b") as s74
┃   ┃           ┇   from "t2"
┃   ┃           ┇   where "a" = 1
┃   ┃           ┇ ) s86, (
┃   ┃           ┇   select avg("b") as s76
┃   ┃           ┇   from "t2"
┃   ┃           ┇   where "a" = 1
┃   ┃           ┇ ) s87

IOW, it works and is decently nice and efficient but there is still no unification.

@hvesalai hvesalai removed the 1 - Ready label Mar 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants