Re: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

On Wed, Mar 2, 2016 at 5:41 AM, Magnus Hagander wrote: > Ok, I've pushed a code that does that. Thank you. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

On Wed, Mar 2, 2016 at 7:34 AM, Michael Paquier wrote: > On Wed, Mar 2, 2016 at 9:19 PM, Magnus Hagander > wrote: > > Needs Review -> Needs Review > > Waiting on Author -> Refuse moving > > Ready for committer -> Ready for Committer > > Committed

Re: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

On Wed, Mar 2, 2016 at 9:19 PM, Magnus Hagander wrote: > Needs Review -> Needs Review > Waiting on Author -> Refuse moving > Ready for committer -> Ready for Committer > Committed -> refuse moving > Moved to next cf -> refuse moving (if it's already set like this, it would >

Re: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

On Tue, Mar 1, 2016 at 10:27 AM, Tom Lane wrote: > Alvaro Herrera writes: > > Magnus Hagander wrote: > >> That said, we can certainly reconsider that. Would we always copy the > value > >> over? Even if it was, say, rejected? (so it would be copied

Re: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

On Tue, Mar 1, 2016 at 7:27 AM, Tom Lane wrote: > +1 for not moving such patches to the new CF until the author does > something --- at which point they'd change to "Needs Review" state. > But we should not change them into that state without author input. > And I don't see

Re: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

Alvaro Herrera writes: > Magnus Hagander wrote: >> That said, we can certainly reconsider that. Would we always copy the value >> over? Even if it was, say, rejected? (so it would be copied to the new CF >> but still marked rejected) Or is there a subset of behaviors

Re: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

Magnus Hagander wrote: > On Tue, Feb 16, 2016 at 11:12 PM, Pavel Stehule > wrote: > > This behave is pretty unpleasant and frustrating. > > Well, it's in no way a bug, because it's the behavior we agreed upon at the > time :) I guess the "move to next CF" operation is

Re: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

On Tue, Feb 16, 2016 at 11:12 PM, Pavel Stehule wrote: > > > 2016-02-17 3:19 GMT+01:00 Jim Nasby : > >> On 2/16/16 12:38 AM, Michael Paquier wrote: >> >>> When a patch with status "Ready for committer" on CF N is moved to CF >>> (N+1), its

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Wed, Feb 17, 2016 at 2:12 AM, Robert Haas wrote: > Committed, except I left out one comment hunk that I wasn't convinced about. Thank you. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Wed, Dec 23, 2015 at 12:19 PM, Peter Geoghegan wrote: > On Tue, Dec 22, 2015 at 2:59 PM, Robert Haas wrote: >> OK, so I've gone ahead and committed and back-patched that. Can you >> please rebase and repost the remainder as a 9.6 proposal? > > I attach

Re: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

2016-02-17 3:19 GMT+01:00 Jim Nasby : > On 2/16/16 12:38 AM, Michael Paquier wrote: > >> When a patch with status "Ready for committer" on CF N is moved to CF >> (N+1), its status is automatically changed to "Needs Review". That's >> something to be aware of when

Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)

On 2/16/16 12:38 AM, Michael Paquier wrote: When a patch with status "Ready for committer" on CF N is moved to CF (N+1), its status is automatically changed to "Needs Review". That's something to be aware of when cleaning up the CF app entries. I agree with Alvarro; this seems like a bug to

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Tue, Feb 16, 2016 at 5:58 AM, Alvaro Herrera wrote: > Peter Geoghegan wrote: > >> Michael (the CF manager at the time) remembered to change the status >> to "Ready for Committer" again; you see this entry immediately >> afterwards: >> >> "New status: Ready for

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

Alvaro Herrera writes: > (FWIW I'm not the "current" CF manager, because the CF I managed is > already over. I'm not sure that we know who the manager for the > upcoming one is.) We had a vict^H^H^H^Hvolunteer a few days ago:

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Mon, Feb 15, 2016 at 12:58 PM, Alvaro Herrera wrote: > (FWIW I'm not the "current" CF manager, because the CF I managed is > already over. I'm not sure that we know who the manager for the > upcoming one is.) It's pretty easy to forget that this was the first time

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

Peter Geoghegan wrote: > Michael (the CF manager at the time) remembered to change the status > to "Ready for Committer" again; you see this entry immediately > afterwards: > > "New status: Ready for Committer" > > but I gather from the CF app history that Alvaro (the current CF > manager) did

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Tue, Dec 22, 2015 at 10:49 PM, Peter Geoghegan wrote: > I attach a rebased patch for 9.6 only. I marked the patch -- my own patch -- "Ready for Committer". I'm the third person to have marked the patch "Ready for Committer", even though no committer bounced the patch back for

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Tue, Dec 22, 2015 at 10:49 AM, Peter Geoghegan wrote: > On Tue, Dec 22, 2015 at 9:31 AM, Robert Haas wrote: >>> It has some nice properties, because unsigned integers are so simple >>> and flexible. For example, we can do it with int4 sometimes, and

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Tue, Dec 22, 2015 at 9:31 AM, Robert Haas wrote: >> It has some nice properties, because unsigned integers are so simple >> and flexible. For example, we can do it with int4 sometimes, and then >> compare two attributes at once on 64-bit platforms. Maybe the second >>

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Tue, Dec 22, 2015 at 12:31 PM, Robert Haas wrote: > On Mon, Dec 21, 2015 at 2:55 PM, Peter Geoghegan wrote: >> On Mon, Dec 21, 2015 at 10:53 AM, Robert Haas wrote: >>> PFA my proposal for comment changes for 9.5 and master. This

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Tue, Dec 22, 2015 at 2:59 PM, Robert Haas wrote: >> Right. I don't think that we should back-patch that stuff into 9.5. > > OK, so I've gone ahead and committed and back-patched that. Can you > please rebase and repost the remainder as a 9.6 proposal? OK. I don't know

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Tue, Dec 22, 2015 at 2:59 PM, Robert Haas wrote: > OK, so I've gone ahead and committed and back-patched that. Can you > please rebase and repost the remainder as a 9.6 proposal? I attach a rebased patch for 9.6 only. -- Peter Geoghegan From

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Mon, Dec 21, 2015 at 2:55 PM, Peter Geoghegan wrote: > On Mon, Dec 21, 2015 at 10:53 AM, Robert Haas wrote: >> PFA my proposal for comment changes for 9.5 and master. This is based >> on your 0001, but I edited somewhat. Please let me know your >>

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Mon, Dec 21, 2015 at 10:53 AM, Robert Haas wrote: > PFA my proposal for comment changes for 9.5 and master. This is based > on your 0001, but I edited somewhat. Please let me know your > thoughts. I am not willing to go further and rearrange actual code in > 9.5 at

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Fri, Dec 18, 2015 at 2:22 PM, Peter Geoghegan wrote: > On Fri, Dec 18, 2015 at 9:35 AM, Robert Haas wrote: >>> Attached revision updates both the main commit (the optimization), and >>> the backpatch commit (updated the contract). >> >> - /*

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Thu, Dec 17, 2015 at 11:43 PM, Peter Geoghegan wrote: > On Wed, Dec 16, 2015 at 12:04 PM, Peter Geoghegan wrote: >>> What kind of state is that? Can't we define this in terms of what it >>> is rather than how it gets that way? >> >> It's zeroed. >> >> I

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Fri, Dec 18, 2015 at 9:35 AM, Robert Haas wrote: >> Attached revision updates both the main commit (the optimization), and >> the backpatch commit (updated the contract). > > - /* abbreviation is possible here only for by-reference types */ > + /* > +

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Wed, Dec 16, 2015 at 12:04 PM, Peter Geoghegan wrote: >> What kind of state is that? Can't we define this in terms of what it >> is rather than how it gets that way? > > It's zeroed. > > I guess we can update everything, including existing comments, to reflect > that.

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Wed, Dec 9, 2015 at 5:15 PM, Peter Geoghegan wrote: > On Wed, Dec 9, 2015 at 11:31 AM, Robert Haas wrote: >> I find the references to a "void" representation in this patch to be >> completely opaque. I see that there are some such references in >>

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Wed, Dec 16, 2015 at 9:36 AM, Robert Haas wrote: >> That isn't what is intended. "void" is the state that macros like >> index_getattr() leave NULL leading attributes (that go in the >> SortTuple.datum1 field) in. > > What kind of state is that? Can't we define this in

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Wed, Dec 9, 2015 at 2:15 PM, Peter Geoghegan wrote: > I think that you're missing that patch 0001 formally forbids > abbreviated keys that are pass-by-value, by revising the contract > (this is proposed for backpatch to 9.5 -- only comments are changed). > This is already

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Sat, Oct 10, 2015 at 9:03 PM, Peter Geoghegan wrote: > On Fri, Sep 25, 2015 at 2:39 PM, Jeff Janes wrote: >> This needs a rebase, there are several conflicts in >> src/backend/executor/nodeAgg.c > > I attached a revised version of the second patch in

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Wed, Dec 9, 2015 at 11:31 AM, Robert Haas wrote: > I find the references to a "void" representation in this patch to be > completely opaque. I see that there are some such references in > tuplesort.c already, and most likely they were put there by commits > that I did,

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Wed, Dec 9, 2015 at 2:15 PM, Peter Geoghegan wrote: > I think that you're missing that patch 0001 formally forbids > abbreviated keys that are pass-by-value Sorry. I do of course mean it forbids abbreviated keys that are *not* pass-by-value (that are pass-by-reference). --

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

Hi, I have reviewed this patch and it still applies to master, compiles and passes the test suite. I like the goal of the patch, making use of the already existing abbreviation machinery in more cases is something we should do and the patch itself looks clean. I can also confirm the

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Sun, Dec 6, 2015 at 7:14 PM, Andreas Karlsson wrote: > I can also confirm the roughly 25% speedup in the best case (numerics which > are all distinct) with no measurable slowdown in the worst case. > > Given this speedup and the small size of the patch I think we should

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Fri, Sep 25, 2015 at 2:39 PM, Jeff Janes wrote: > This needs a rebase, there are several conflicts in > src/backend/executor/nodeAgg.c I attached a revised version of the second patch in the series, fixing this bitrot. I also noticed a bug in tuplesort's

[HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

This needs a rebase, there are several conflicts in src/backend/executor/nodeAgg.c Thanks, Jeff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers