Re: [PATCH] explain sortorder

On 01/13/2015 06:04 PM, Timmer, Marius wrote:
>   -malloc() (StringInfo is used as suggested now).

There really shouldn't be any snprintf() calls in the patch, when 
StringInfo is used correctly...

> @@ -1187,6 +1187,7 @@ explain (verbose, costs off) select * from matest0 order by 1-id;
>   Sort
>     Output: matest0.id, matest0.name, ((1 - matest0.id))
>     Sort Key: ((1 - matest0.id))
> +   Sort Order: ASC NULLS LAST
>     ->  Result
>           Output: matest0.id, matest0.name, (1 - matest0.id)
>           ->  Append

This patch isn't going to be committed with this output format. Please 
change per my suggestion earlier:

> I don't like this output. If there are a lot of sort keys, it gets
> difficult to match the right ASC/DESC element to the sort key it applies
> to. (Also, there seems to be double-spaces in the list)
>
> I would suggest just adding the information to the Sort Key line. As
> long as you don't print the modifiers when they are defaults (ASC and
> NULLS LAST), we could print the information even in non-VERBOSE mode. So
> it would look something like:
>
> Sort Key: sortordertest.n1 NULLS FIRST, sortordertest.n2 DESC

Or if you don't agree with that, explain why.

- Heikki