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