Add reorder function

[scttnlsn]

This pull-request adds a reorder function similar to the existing order function, however, it first clears any existing order clauses.

Example:

(def query (-> (select* users)
               (order :foo)
               (order :bar)))

(exec (-> query
          (reorder :created))

This results in the following SQL:

SELECT "users".* FROM "users" ORDER BY "users"."created" ASC

---

As another example, consider the case where we have a sorted base query that we'd like to return along with the total count. In the following case the reorder is actually necessary (otherwise the count query fails).

(def query (-> (select* users)
               (where {:username [= "foo"])
               (order :created)))

(def results (-> query
                 (limit 10)
                 (offset 25)))

(def total (-> query
               (aggregate (count :*) :total)
               (reorder :total)))

[immoh]

I don't really see the need for this. Wouldn't it be simpler to omit order from base query?

(def query (-> (select* users)
               (where {:username [= "foo"])))

(def results (-> query
                 (order :created)
                 (limit 10)
                 (offset 25)))

(def total (-> query
               (aggregate (count :*) :total)
               (order :total)))

[scttnlsn]

My use case that is motivating the need for a reorder function is a reusable pagination component. It accepts a query (and paging options) and returns two values:

1. The paginated results (i.e. the query w/ added limit and offset clauses)
2. The aggregate count of all the records matching the given query

I don't think the pagination component should need to care about whether the initial query is sorted or not. Since the count query cannot be ordered by an arbitrary field, the only alternative that I'm aware of is to pass the unordered query to the pagination component and then perform any sorting on the paged result query. I'd argue that needing to know the proper call order like this is more complex than using the reorder.

I'm currently clearing the previous order clauses manually:

(-> query
    (assoc :order [])
    (order :total))

But it would be great if there was a function to do this so my application doesn't need to reach into the internal query data structure (which doesn't seem to be part of Korma's public API).

[bitemyapp]

A nomenclature note, the term "reorder" was a bit confusing because you're using it to reset ordering if I understand the comments correctly.

[scttnlsn]

I just stole the name from ActiveRecord (http://apidock.com/rails/ActiveRecord/QueryMethods/reorder). You're understanding it correctly though, it does reset all existing order clauses. Perhaps another name is more apt.

[bitemyapp]

@scttnlsn perhaps reset-order? Don't leap into it making changes on my account, decision on this PR rests with @immoh.

[immoh]

In your use case you will still run into trouble if the initial query selects only certain columns using fields and you would need a reset function for fields as well. I don't feel comfortable about introducing these reset functions. I think queries should be composed from smaller units instead of resetting parts of them. Also your pagination component probably should not take in query as argument but something that you can compose both queries from.

[scttnlsn]

@immoh That's a good point. Personally, I think the query is a nice level of abstraction for this sort of thing but I understand your reluctance to add more reset functions. If you were writing a pagination component, what would you pass instead of the query? A collection of query clauses? What if one of those clauses is an ordering? Regardless of how the passed query is represented, I see two high-level approaches:

1. The pagination component accepts any query and re-orders/re-selects as needed. This way the caller doesn't need to know how the paginator is implemented.
2. The pagination component assumes that the given query is not sorted. The caller needs to be aware of this and should only compose the ordering/selection into a query returned from the paginator.

Option 1 seems nicer to me. Is there a way to do that w/o reordering?

[bilus]

The count returned by the paginator, why cannot it use ORDER BY? AFAIK it's valid SQL, isn't it?

[scttnlsn]

@bilus I don't think so. Here's the error I get from Postgres when executing the following query:

SELECT COUNT(*) FROM users ORDER BY id;

ERROR:  column "users.id" must appear in the GROUP BY clause or be used in an aggregate function
LINE 1: SELECT COUNT(*) FROM users ORDER BY id;
                                            ^

[D-side]

@bilus it isn't. Moreover, it makes no sense: column id does not appear in the result set, so it can't be ordered by it. It only has one column, count, and only one row (so, actually, any ordering makes no sense).

@scttnlsn I see two possible options for this use case, without reorder:

1. Separate order-clause from the rest of the query and pass it in as another argument. Korma can do that, you can define orders in a function and thread-first through it just the same.

   Your proposal is basically to allow passing in both as a single argument and have order-clauses cut out where not necessary. This is somewhat the same, but requires a way to manipulate existing order-clauses, which is what reorder does. I personally think it's overly hacky: there may be not only order that can break count-query, but I haven't explored what else can.

2. Use something like:

   SELECT COUNT(*) FROM (SELECT whatever FROM whatever ORDER BY whatever);

   It makes sense and almost works in Postgres, but it also requires the subquery in FROM to have an alias, like:

     SELECT COUNT(*) FROM (SELECT whatever FROM whatever ORDER BY whatever) whatever;

   I haven't found a way to build such a query in Korma except defining an entity from subselect with order baked in. A page fetch on such an entity would generate something awkward (yet valid):

     SELECT * FROM
       (SELECT * FROM whatever ORDER BY whatever) whatever
     OFFSET 75 LIMIT 25

   Still, the COUNT query is well able to measure a result set of any valid query, but since it involves some indirection it may not be as efficient performance-wise. I don't know though. On my tiny dataset Postgres outputs identical query plans and costs, so maybe it's okay.

I've been using Rails for a while now. Rails' default ORM, ActiveRecord, is built on top of Arel, which is a project similar to Korma in Ruby world. And reorder comes from ActiveRecord, not Arel. So just as there's no reorder in SQL or Arel, I don't think it's needed in Korma.

ActiveRecord itself has a certain abstract on top of Arel queries. Basically, a map that contains values fed in by various methods like order, select, where. When execution is requested, these are transformed into Arel AST and then executed. Perhaps you need a different abstract as well. Korma's "entity" is one of the possible ways, if you consider the types of queries listed above as "mostly fine".