Daniel,
I took a look at the patch. It applies and compiles, the tests pass.
Some thoughts about the code:
* Postgres lists cache their lengths, so you don't need uniqueLen.
* Use an array of WindowClauseSortNode's instead of a list. It's more
suitable here because you are going to sort (qsort_list creates a
temporary array).
* Reversing the sorted list looks more confusing to me than just sorting
it in the proper order right away. A qsort() comparator returning
negative means "left goes before right", but here it is effectively the
other way around.
* This isn't relevant given the previous points, but to reverse a list,
you can walk it with foreach() and construct a reversed version with
lcons().
* There is a function named make_pathkeys_for_window that makes a list
of canonical pathkeys given a window clause. Using this function to give
you the pathkeys, and then comparing them, would be more future-proof in
case we ever start using hashing for windowing. Moreover, the canonical
pathkeys can be compared with pointer comparison which is much faster
than equal(). Still, I'm not sure whether it's going to be convenient to
use in this case, or even whether it is a right thing to do. What do you
think?
--
Alexander Kuzmenkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
