"Timmer, Marius" <marius.timmer@uni-muenster.de> writes:
> attached is version 8, fixing remaining issues, adding docs and tests as requested/agreed.
I've committed this with some rather substantial alterations, notably:
* Having get_sortorder_by_keyno dig into the plan state node by itself
seemed like a bad idea; it's certainly at variance with the existing
division of knowledge in this code, and I think it might outright do
the wrong thing if there's a Sort node underneath an Agg or Group node
(since in those cases the child Sort node, not the parent node, would
get passed in). I fixed it so that show_sort_keys and siblings are
responsible for extracting and passing in the correct data arrays.
* There were some minor bugs in the rules for when to print NULLS
FIRST/LAST too. I think the way I've got it now is a precise inverse of
what addTargetToSortList() will do.
* The proposed new regression test cases were not portable ("en_US"
isn't guaranteed to exist), and I thought adding a new regression
script file for just one test was a bit excessive. The DESC and
USING behaviors were already covered by existing test cases so I just
added something to exercise COLLATE and NULLS FIRST in collate.sql.
* I took out the change in perform.sgml. The change as proposed was
seriously confusing because it injected off-topic discussion into an
example that was meant to be just about the additional information printed
by EXPLAIN ANALYZE. I'm not really sure that this feature needs any
specific documentation (other than its eventual mention in the release
notes); but if it does, we should probably add a brand new example
someplace before the EXPLAIN ANALYZE subsection.
* Assorted cleanups such as removal of irrelevant whitespace changes.
That sort of thing just makes a reviewer's job harder, so it's best
to avoid it if you can.
regards, tom lane