Ecto: Support NULLS LAST in order_by

[mspanc]

Environment

• Elixir version (elixir -v): 1.2.1
• Database and version (PostgreSQL 9.4, MongoDB 3.2, etc.): 9.4.5
• Ecto version (mix deps): 1.1.8
• Database adapter and version (mix deps): postgrex 0.11.1
• Operating system: ubuntu 14.04
  

  Current behavior

There's no DSL that will map into postgresql's NULLS [LAST | FIRST] in order_by clause

Expected behavior

There is a DSL that will map into postgresql's NULLS [LAST | FIRST] in order_by clause

[michalmuskala]

While we don't have this in the DSL, it can be easily achieved with fragments:

from f in Foo, order_by: [asc: fragment("? NULLS LAST", f.bar)]

You could also create a simple macro wrapper to make it easier, if you use this a lot:

defmacro nulls_last(field) do
  quote do
    fragment("? NULLS LAST", unquote(field))
  end
end

from f in Foo, order_by: [asc: nulls_last(f.bar)]

I'm not sure we should add this to the DSL.

/cc @josevalim

[josevalim]

@michalmuskala is the nulls_last per field? If so I think your solution is beautiful and perfectly fine.

[ericmj]

I use a fragment in hex_web https://github.com/hexpm/hex_web/blob/86eb0a71ad2322b329dba6197f91b6aa59abed5e/web/models/package.ex#L165

[michalmuskala]

@josevalim yes, it's per field.

I had the snipper ready, because I wrote it in one of my projects just yesterday 😄

[nathanl]

@michalmuskala Your fragment example helped me, but I had to modify it slightly (I'm using Ecto 2.1.4 - maybe that matters?).

I'm using order_by: [fragment("? DESC NULLS LAST", foo.bar], which produces SQL ORDER BY foo."bar" DESC NULLS LAST as it should.

If I do order_by: [desc: fragment("? NULLS LAST", f.bar)], it causes a SQL syntax error because the generated SQL is ORDER BY foo."bar" NULLS LAST DESC. The DESC must come before NULLS LAST.

[michalmuskala]

Yes, you're right. I think it never worked as it's described there.

I guess we could support it in ecto using order_by: [desc_nulls_last: foo, asc_nulls_first: bar], I'm not 100% sure we want to add this to ecto itself, though

[nathanl]

I don't think it's necessary. I mostly wanted to leave a breadcrumb for myself, for the next time I Google this. :)