Re: [HACKERS] [PATCH] explain sortorder

2015-01-19 Thread Timmer, Marius
Hi Tom, we are very happy seeing this committed. Thank you for committing and Mike for the review!! Your changes make sense to us, except one question: We think, you wanted to switch to DESC behavior (print out NULLS FIRST) in cases, where „USING“ uses an operator which is considered to be a

Re: [HACKERS] [PATCH] explain sortorder

2015-01-19 Thread Tom Lane
Timmer, Marius marius.tim...@uni-muenster.de writes: We think, you wanted to switch to DESC behavior (print out NULLS FIRST) in cases, where „USING“ uses an operator which is considered to be a DESC operator. Right, because that's how addTargetToSortList() would parse it. But

Re: [HACKERS] [PATCH] explain sortorder

2015-01-19 Thread Mike Blackwell
Tom, Thanks for the comments on what you ended up changing. It helps point out the kind of things I should be looking for. I'll try to let less slip through in the future. Mike __ *Mike Blackwell | Technical

Re: [HACKERS] [PATCH] explain sortorder

2015-01-16 Thread Tom Lane
Timmer, Marius marius.tim...@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

Re: [HACKERS] [PATCH] explain sortorder

2015-01-15 Thread Tom Lane
Timmer, Marius marius.tim...@uni-muenster.de writes: attached is version 8, fixing remaining issues, adding docs and tests as requested/agreed. I'll pick this up --- I've been a bit lax about helping with this commitfest. regards, tom lane -- Sent via pgsql-hackers

Re: [HACKERS] [PATCH] explain sortorder (fwd)

2015-01-15 Thread Mike Blackwell
From: Timmer, Marius marius.tim...@uni-muenster.de Hi, attached is version 8, fixing remaining issues, adding docs and tests as requested/agreed. Marius Arne ​This looks good to me. Test coverage seems complete. Doc updates are included. Output format looks like it should be

Re: [HACKERS] [PATCH] explain sortorder

2015-01-15 Thread Timmer, Marius
Hi, attached is version 8, fixing remaining issues, adding docs and tests as requested/agreed. Marius Arne --- Marius Timmer Zentrum für Informationsverarbeitung Westfälische Wilhelms-Universität Münster Einsteinstraße 60 mtimm...@uni-muenster.de Am 14.01.2015 um 17:42 schrieb Arne

Re: [HACKERS] [PATCH] explain sortorder

2015-01-14 Thread Arne Scheffer
Hi, we will also remove the following is lc_collate hint in the next version, showing only mandatory info as suggested. /* for those who use COLLATE although their default is already the wanted */ if (strcmp(collname, localeptr) == 0) {

Re: [HACKERS] [PATCH] explain sortorder

2015-01-14 Thread Timmer, Marius
Hello Heikki, abbreviated version: Sorry, the problem is only the unhandy patch text format, not different opinions how to proceed. Long version: The v7 patch file already addressed your suggestions, but the file contained serveral (old) local commits, the new ones at the end of the patch

Re: [HACKERS] [PATCH] explain sortorder

2015-01-14 Thread Heikki Linnakangas
On 01/14/2015 05:26 PM, Timmer, Marius wrote: Hello Heikki, abbreviated version: Sorry, the problem is only the unhandy patch text format, not different opinions how to proceed. Long version: The v7 patch file already addressed your suggestions, but the file contained serveral (old) local

Re: [HACKERS] [PATCH] explain sortorder

2015-01-13 Thread Heikki Linnakangas
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

Re: [HACKERS] [PATCH] explain sortorder

2015-01-13 Thread Timmer, Marius
Hi, we removed -malloc() (StringInfo is used as suggested now). -leftover commented out code -the splitting of existing declaration and initialization in show_group_keys(). Missing tests and documentation are WIP and will follow with the next patch version. Best regards Marius ---

Re: [HACKERS] [PATCH] explain sortorder

2015-01-07 Thread Timmer, Marius
Hi, we have spent the last days to realize your suggestions in the patch. It affects the result of a EXPLAIN-Statement (even in non-verbose-mode). Now you will get the order-information for every single sort-key which is not ordered by the defaults. best regards, Marius --- Marius Timmer

Re: [HACKERS] [PATCH] explain sortorder

2015-01-07 Thread Mike Blackwell
V6 of this patch applies, builds and checks against the current HEAD. The areas below could use some attention. In explain.c: malloc() should not be called directly here. palloc() would be the correct call, I believe, but the functions in stringinfo.h are probably your best choice as they

Re: [HACKERS] [PATCH] explain sortorder

2014-12-26 Thread Arne Scheffer
Heikki Linnakangas hlinnakangas(at)vmware(dot)com writes: 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. +1. I had assumed without

Re: [HACKERS] [PATCH] explain sortorder

2014-12-23 Thread Mike Blackwell
Looking forward to the new patch. I'll give it a more complete testing when you post it. Mike __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St

Re: [HACKERS] [PATCH] explain sortorder

2014-12-17 Thread Heikki Linnakangas
On 12/15/2014 06:49 PM, Mike Blackwell wrote: QUERY PLAN Sort Output: n1, n2, ((n1)::character(1)) Sort Key: sortordertest.n1, sortordertest.n2 Sort Order: ASC NULLS LAST, ASC NULLS LAST - Seq Scan on

Re: [HACKERS] [PATCH] explain sortorder

2014-12-17 Thread Tom Lane
Heikki Linnakangas hlinnakan...@vmware.com writes: 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. +1. I had assumed without

Re: [HACKERS] [PATCH] explain sortorder

2014-12-16 Thread Timmer, Marius
Hi Mike, Now I've replaced the spaces at the beginning of the lines with tabs. Quotes() in the expected/explain_sortorder.out-File caused failing „make check“. So I changed them to single quotes('). I’ve added the corrected patch as attachment. Kind regards, Marius

Re: [HACKERS] [PATCH] explain sortorder

2014-12-15 Thread Mike Blackwell
Initial review: Patch applies cleanly to current head, although it appears to have soft/hard tab and trailing space issues. make check fails with the output below. The expected collation clause is not present. -- -- Test explain feature: sort order -- CREATE TABLE sortordertest (n1 char(1), n2