> On 13 Sep 2018, at 19:50, Andrew Gierth wrote:
>
> Here's what I have queued up to push.
LGTM, thanks!
> + * framing clauses differ), then all peer rows must be presented in the
> + * same order in all of them. If we allowed multiple sort nodes for such
Should probably be
Here's what I have queued up to push.
My changes are:
- added to the header comment of list_concat_unique that callers have
ordering expectations. Didn't touch list_union, since I ended up
sticking with list_concat_unique for this patch.
- WindowClauseSortNode renamed
Andrew Gierth writes:
> (aside: I itch to rewrite the comment that says "the spec requires that
> there be only one sort" - number of sorts is an implementation detail
> about which the spec is silent, what it _actually_ requires is that peer
> rows must be presented in the same order in all
> "Tom" == Tom Lane writes:
Tom> I'm also a bit suspicious as to whether the code is even correct;
Tom> does it *really* match what will happen later when we create sort
Tom> plan nodes? (Maybe it's fine; I haven't looked.)
As things stand before the patch, the code that actually
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> * I'm almost thinking that changing to list_union is a bad idea,
> A fair point. Though it looks like list_union is used in only about 3
> distinct places, and two of those are list_union(NIL, blah) to simply
> remove dups from a single
> "Tom" == Tom Lane writes:
Tom> * I'm almost thinking that changing to list_union is a bad idea,
A fair point. Though it looks like list_union is used in only about 3
distinct places, and two of those are list_union(NIL, blah) to simply
remove dups from a single list. The third place is
Andrew Gierth writes:
> So I'm looking to commit this, and here's my comments so far:
I took a quick look over this. I agree with your nitpicks, and have
a couple more:
* Please run it through pgindent. That will, at a minimum, remove some
gratuitous whitespace changes in this patch. I think
> On 12 Sep 2018, at 22:15, Andrew Gierth wrote:
> WindowClauseSortNode - I don't like this name, because it's not actually
> a Node of any kind. How about WindowSortData?
That’s a good point. I probably would’ve named it WindowClauseSortData since
it acts on WindowClauses, but that might just
So I'm looking to commit this, and here's my comments so far:
WindowClauseSortNode - I don't like this name, because it's not actually
a Node of any kind. How about WindowSortData?
list_concat_unique(list_copy(x),y) is exactly list_union(x,y), which
looks a bit nicer to me.
re. this:
for
On Sat, Jul 28, 2018 at 4:12 AM, Alexander Kuzmenkov
wrote:
> Daniel,
>
> Thanks for the update.
>
>
> On 07/25/2018 01:37 AM, Daniel Gustafsson wrote:
>>
>>
>>> Hmm, this is missing the eqop fields of SortGroupClause. I haven't
>>> tested yet but does the similar degradation happen if two
>>>
The last version looked OK, so I'm marking this patch as ready for
committer in the commitfest app.
--
Alexander Kuzmenkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
> On 27 Jul 2018, at 21:12, Alexander Kuzmenkov
> wrote:
> Thanks for the update.
Thank you for reviewing and hacking!
> On 07/25/2018 01:37 AM, Daniel Gustafsson wrote:
>>
>>> Hmm, this is missing the eqop fields of SortGroupClause. I haven't
>>> tested yet but does the similar degradation
Daniel,
Thanks for the update.
On 07/25/2018 01:37 AM, Daniel Gustafsson wrote:
Hmm, this is missing the eqop fields of SortGroupClause. I haven't
tested yet but does the similar degradation happen if two
SortGroupCaluses have different eqop and the same other values?
I wasn’t able to
> On 3 Jul 2018, at 12:24, Masahiko Sawada wrote:
> Thank you for updating the patch.
Thanks for reviewing!
> Hmm, this is missing the eqop fields of SortGroupClause. I haven't
> tested yet but does the similar degradation happen if two
> SortGroupCaluses have different eqop and the same other
On Tue, Jul 3, 2018 at 6:19 AM, Daniel Gustafsson wrote:
>> On 2 Jul 2018, at 14:01, Masahiko Sawada wrote:
>
>> Thank you for updating the patch! There are two review comments.
>
> Thanks for reviewing!
>
>> The current select_active_windows() function compares the all fields
>> of WindowClause
> On 2 Jul 2018, at 14:01, Masahiko Sawada wrote:
> Thank you for updating the patch! There are two review comments.
Thanks for reviewing!
> The current select_active_windows() function compares the all fields
> of WindowClause for the sorting but with this patch we compare only
>
On Mon, Jul 2, 2018 at 5:25 PM, Daniel Gustafsson wrote:
>> On 26 Jun 2018, at 17:11, Alexander Kuzmenkov
>> wrote:
>
>> I took a look at the patch. It applies and compiles, the tests pass.
>
> Thanks for reviewing, and apologies for the slow response.
>
>> Some thoughts about the code:
>>
>> *
> On 26 Jun 2018, at 17:11, Alexander Kuzmenkov
> wrote:
> I took a look at the patch. It applies and compiles, the tests pass.
Thanks for reviewing, and apologies for the slow response.
> Some thoughts about the code:
>
> * Postgres lists cache their lengths, so you don't need uniqueLen.
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
> On 30 May 2018, at 18:19, Daniel Gustafsson wrote:
>
> Currently, we can only reuse Sort nodes between WindowAgg nodes iff the
> partitioning and ordering clauses are identical. If a window Sort node
> sortorder is a prefix of another window, we could however reuse the Sort node
> to
Currently, we can only reuse Sort nodes between WindowAgg nodes iff the
partitioning and ordering clauses are identical. If a window Sort node
sortorder is a prefix of another window, we could however reuse the Sort node
to hopefully produce a cheaper plan. In
21 matches
Mail list logo
