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

SELECT DISTINCT ON expressions must match initial ORDER BY expressions #495

Closed
gjaldon opened this issue Mar 3, 2015 · 9 comments
Closed

Comments

@gjaldon
Copy link
Contributor

gjaldon commented Mar 3, 2015

I encounter the error SELECT DISTINCT ON expressions must match initial ORDER BY expressions
for a query that looks like this:

from(f in query,
  distinct: fragment("?[?]", f.path, ^level),
  order_by: [asc: fragment("?[?]", f.path, ^level), desc: f.id])

Which outputs the following query in Postgres:

SELECT DISTINCT ON (f0."path"[$1]) f0."id", f0."path", f0."salt", f0."hash", f0."size", f0."mtime", f0."atime", f0."ctime", f0."content", f0."removed", f0."created_at", f0."updated_at", f0."user_id" FROM "files" AS f0 ORDER BY f0."path"[$2], f0."id" DESC [2, 2]

It works when I change the query to:

from(f in query,
  distinct: fragment("?[?]", f.path, ^level),
  order_by: [asc: fragment("?[$1]", f.path), desc: f.id])

I just hardcode $1 to the above query so the output query in Postgres is:

SELECT DISTINCT ON (f0."path"[$1]) f0."id", f0."path", f0."salt", f0."hash", f0."size", f0."mtime", f0."atime", f0."ctime", f0."content", f0."removed", f0."created_at", f0."updated_at", f0."user_id" FROM "files" AS f0 ORDER BY f0."path"[$1], f0."id" DESC
@josevalim
Copy link
Member

@ericmj what if we automatically set the order_by when we set the distinct? Or we introduce something like distinct_order_by? It seems silly to force the user to repeat the expressions?

@ericmj
Copy link
Member

ericmj commented Mar 3, 2015

@josevalim Yes, but it also seems like we should try to converge when the same expression is interpolated twice to a single query parameter.

@josevalim
Copy link
Member

@ericmj how should we do it though? check for equality in the params as we merge them? Unfortunately it doesn't work for database like mysql. should it be a postgres connection optimization?

@ericmj
Copy link
Member

ericmj commented Mar 4, 2015

I didn't consider mysql so lets table that. It's useful for optimization but not for anything else.

@josevalim
Copy link
Member

@ericmj should we then change the postgres adapter to automatically add what is in distinct to order_by?

@ericmj
Copy link
Member

ericmj commented Mar 4, 2015

I'm not sure, maybe it should be explicit? Should we do it by default even though Postgres doesn't?

@josevalim
Copy link
Member

I am not sure. We will definitely have issues like above AND there is no work around. Hardcoding to $1 is brittle. :(

@josevalim
Copy link
Member

I think we should give it a go at automatically copying. Let's just make it explicit in the docs. So this is now ready for tackling. /cc @briksoftware @whatyouhide

@gjaldon
Copy link
Contributor Author

gjaldon commented Mar 5, 2015

I'd like to take this one on. Expect a PR in the next 24 hours. :)

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

3 participants