Re: [HACKERS] Combining Aggregates

On 9 April 2016 at 05:49, Robert Haas wrote: > On Fri, Apr 8, 2016 at 11:57 AM, Robert Haas wrote: >> On Wed, Apr 6, 2016 at 5:28 PM, David Rowley >> wrote: Is that everything now? I don't see anything about combining aggs in the git log and this is still showing as UnCommitted i

Re: [HACKERS] Combining Aggregates

On Fri, Apr 8, 2016 at 11:57 AM, Robert Haas wrote: > On Wed, Apr 6, 2016 at 5:28 PM, David Rowley > wrote: >>> Is that everything now? I don't see anything about combining aggs in the git >>> log and this is still showing as UnCommitted in the CF app. >> >> There's just a documentation patch and

Re: [HACKERS] Combining Aggregates

On Wed, Apr 6, 2016 at 5:28 PM, David Rowley wrote: >> Is that everything now? I don't see anything about combining aggs in the git >> log and this is still showing as UnCommitted in the CF app. > > There's just a documentation patch and two combine functions for > floating point aggs left now (Ha

Re: [HACKERS] Combining Aggregates

On 6 April 2016 at 22:28, David Rowley wrote: > On 7 April 2016 at 09:25, Simon Riggs wrote: > > On 5 April 2016 at 19:33, Robert Haas wrote: > > > >> > >> Committed 0002+0003 with those changes, some minor cosmetic stuff, and > >> of course the obligatory catversion bump. Oh, and fixed an OID

Re: [HACKERS] Combining Aggregates

On 7 April 2016 at 09:25, Simon Riggs wrote: > On 5 April 2016 at 19:33, Robert Haas wrote: > >> >> Committed 0002+0003 with those changes, some minor cosmetic stuff, and >> of course the obligatory catversion bump. Oh, and fixed an OID >> conflict with the patch Magnus just committed. > > > Is

Re: [HACKERS] Combining Aggregates

On 5 April 2016 at 19:33, Robert Haas wrote: > Committed 0002+0003 with those changes, some minor cosmetic stuff, and > of course the obligatory catversion bump. Oh, and fixed an OID > conflict with the patch Magnus just committed. Is that everything now? I don't see anything about combining

Re: [HACKERS] Combining Aggregates

On Tue, Apr 5, 2016 at 2:52 PM, Alvaro Herrera wrote: > Robert Haas wrote: >> Now, let's suppose that the user sets up a sharded table and then >> says: SELECT a, SUM(b), AVG(c) FROM sometab. At this point, what we'd >> like to have happen is that for each child foreign table, we go and >> fetch

Re: [HACKERS] Combining Aggregates

Robert Haas wrote: > Now, let's suppose that the user sets up a sharded table and then > says: SELECT a, SUM(b), AVG(c) FROM sometab. At this point, what we'd > like to have happen is that for each child foreign table, we go and > fetch partially aggregated results. Those children might be runni

Re: [HACKERS] Combining Aggregates

On Tue, Apr 5, 2016 at 10:19 AM, Robert Haas wrote: > I'm going to concede the point that this shouldn't really be a > priority for 9.6, but I might want to come back to it later. It seems to me that if two aggregates are using the same transition function, they ought to also be using the same co

Re: [HACKERS] Combining Aggregates

On Tue, Apr 5, 2016 at 9:30 AM, David Rowley wrote: > On 6 April 2016 at 01:01, Robert Haas wrote: >> On Tue, Apr 5, 2016 at 3:55 AM, David Rowley >>> To be really honest, I'm quite worried that if I go and make this >>> change then my time might be wasted as I really think making that >>> change

Re: [HACKERS] Combining Aggregates

On 6 April 2016 at 01:01, Robert Haas wrote: > On Tue, Apr 5, 2016 at 3:55 AM, David Rowley >> To be really honest, I'm quite worried that if I go and make this >> change then my time might be wasted as I really think making that >> change this late in the day is just setting this up for failure.

Re: [HACKERS] Combining Aggregates

On Tue, Apr 5, 2016 at 3:55 AM, David Rowley wrote: >> I think it might be a good idea if these patches made less use of >> bytea and exposed the numeric transition values as, say, a 2-element >> array of numeric. > > Well, if you have a look at NumericAggState you can see it's not quite > as simp

Re: [HACKERS] Combining Aggregates

On 5 April 2016 at 16:54, Robert Haas wrote: > On Wed, Mar 30, 2016 at 3:34 PM, David Rowley > wrote: >> I wrote 0002 - 0004, these were reviewed by Tomas. >> 0005 is Haribabu's patch: Reviewed by Tomas and I. > > I think it might be a good idea if these patches made less use of > bytea and expos

Re: [HACKERS] Combining Aggregates

On 5 April 2016 at 05:54, Robert Haas wrote: > On Wed, Mar 30, 2016 at 3:34 PM, David Rowley > wrote: > > I wrote 0002 - 0004, these were reviewed by Tomas. > > 0005 is Haribabu's patch: Reviewed by Tomas and I. > > I think it might be a good idea if these patches made less use of > bytea and ex

Re: [HACKERS] Combining Aggregates

On Wed, Mar 30, 2016 at 3:34 PM, David Rowley wrote: > I wrote 0002 - 0004, these were reviewed by Tomas. > 0005 is Haribabu's patch: Reviewed by Tomas and I. I think it might be a good idea if these patches made less use of bytea and exposed the numeric transition values as, say, a 2-element arr

Re: [HACKERS] Combining Aggregates

On 31 March 2016 at 00:48, Robert Haas wrote: > On Tue, Mar 29, 2016 at 11:14 PM, David Rowley > wrote: >>> 0005: >>> Haribabu's patch; no change from last time. > > So what's the distinction between 0002 and 0005? And what is the > correct author/reviewer attribution for each? I wrote 0002 - 0

Re: [HACKERS] Combining Aggregates

On Tue, Mar 29, 2016 at 11:14 PM, David Rowley wrote: > Many thanks Robert for committing the serialize states portion. yw, sorry I didn't get an email out about that. >> 0005: >> Haribabu's patch; no change from last time. So what's the distinction between 0002 and 0005? And what is the corre

Re: [HACKERS] Combining Aggregates

On 26 March 2016 at 15:07, David Rowley wrote: Many thanks Robert for committing the serialize states portion. > 0005: > Haribabu's patch; no change from last time. Just in case you jump ahead. I just wanted to re-highlight something Haribabu mentioned a while ago about combining floating point

Re: [HACKERS] Combining Aggregates

On 26 March 2016 at 15:07, David Rowley wrote: > Ok, so on further look at this I've decided to make changes and have > it so the serialisation function can be dumb about memory contexts in > the same way as finalize_aggregate() allows the final function to be > dumb... notice at the end of the fu

Re: [HACKERS] Combining Aggregates

On 25 March 2016 at 06:17, Robert Haas wrote: > On Mon, Mar 21, 2016 at 2:18 PM, David Rowley > wrote: >> I've attached 2 of the patches which are affected by the changes. > > I think the documentation for 0001 needs some work yet. The > additional paragraph that you've added... > > (1) doesn't

Re: [HACKERS] Combining Aggregates

On Thu, Mar 24, 2016 at 1:17 PM, Robert Haas wrote: > I'm going to read through the code again now. OK, I noticed another documentation problem: you need to update catalogs.sgml for these new columns. +* Validate the serial function, if present. We must ensure that the return +*

Re: [HACKERS] Combining Aggregates

On Mon, Mar 21, 2016 at 2:18 PM, David Rowley wrote: > I've attached 2 of the patches which are affected by the changes. I think the documentation for 0001 needs some work yet. The additional paragraph that you've added... (1) doesn't seem to appear at a very logical place in the documentation

Re: [HACKERS] Combining Aggregates

On Thu, Mar 24, 2016 at 5:22 AM, David Rowley wrote: > On 21 January 2016 at 08:06, Robert Haas wrote: >> I re-reviewed this and have committed most of it with only minor >> kibitizing. A few notes: > > I realised today that the combinefunc is rather undocumented. I've > attached a patch which a

Re: [HACKERS] Combining Aggregates

On 21 January 2016 at 08:06, Robert Haas wrote: > I re-reviewed this and have committed most of it with only minor > kibitizing. A few notes: I realised today that the combinefunc is rather undocumented. I've attached a patch which aims to fix this. Comments are welcome. -- David Rowley

Re: [HACKERS] Combining Aggregates

On 20 March 2016 at 16:48, David Rowley wrote: > I've attached another series of patches: > > 0001: This is the latest Parallel Aggregate Patch, not intended for > review here, but is required for the remaining patches. This patch has > changed quite a bit from the previous one that I posted here,

Re: [HACKERS] Combining Aggregates

On Sat, Mar 19, 2016 at 11:48 PM, David Rowley wrote: > 0002: Adds serial/de-serial function support to CREATE AGGREGATE, > contains minor fix-ups from last version. This looks pretty good, but don't build_aggregate_serialfn_expr and build_aggregate_deserialfn_expr compile down to identical machi

Re: [HACKERS] Combining Aggregates

On Sun, Mar 20, 2016 at 2:23 PM, David Rowley wrote: > > I've had a look over this. I had to first base it on the 0005 patch, > as it seemed like the pg_aggregate.h changes didn't include the > serialfn and deserialfn changes, and an OID was being consumed by > another function I added in patch 00

Re: [HACKERS] Combining Aggregates

Hi, On 03/20/2016 04:48 AM, David Rowley wrote: On 17 March 2016 at 14:25, Tomas Vondra wrote: On 03/16/2016 12:03 PM, David Rowley wrote: Patch 2: This adds the serial/deserial aggregate infrastructure, pg_dump support, CREATE AGGREGATE changes, and nodeAgg.c changes to have it serialise and

Re: [HACKERS] Combining Aggregates

On 18 March 2016 at 13:39, Haribabu Kommi wrote: > On Thu, Mar 17, 2016 at 10:59 PM, Tomas Vondra > wrote: >> Hi, >> >> On 03/17/2016 12:53 PM, David Rowley wrote: >>> >> ... >>> >>> >>> I just had a quick skim over the patch and noticed the naming >>> convention you're using for the combine func

Re: [HACKERS] Combining Aggregates

On 17 March 2016 at 16:30, Haribabu Kommi wrote: > On Wed, Mar 16, 2016 at 10:08 PM, David Rowley > wrote: >> On 16 March 2016 at 23:54, Haribabu Kommi wrote: >>> On Wed, Mar 16, 2016 at 8:34 AM, David Rowley >>> wrote: Yes me too, so I spent several hours yesterday writing all of the

Re: [HACKERS] Combining Aggregates

Hi, On 03/17/2016 12:53 PM, David Rowley wrote: > ... > I just had a quick skim over the patch and noticed the naming convention you're using for the combine function is *_pl, and you have float8_pl. There's already a function named float8pl() which is quite close to what you have. I've been sti

Re: [HACKERS] Combining Aggregates

On Wed, Mar 16, 2016 at 10:08 PM, David Rowley wrote: > On 16 March 2016 at 23:54, Haribabu Kommi wrote: >> On Wed, Mar 16, 2016 at 8:34 AM, David Rowley >> wrote: >>> Yes me too, so I spent several hours yesterday writing all of the >>> combine functions and serialisation/deserialisation that a

Re: [HACKERS] Combining Aggregates

Hi, On 03/16/2016 12:03 PM, David Rowley wrote: On 16 March 2016 at 10:34, David Rowley wrote: If Haribabu's patch does all that's required for the numerical aggregates for floating point types then the status of covered aggregates is (in order of pg_aggregate.h): * AVG() complete coverage *

Re: [HACKERS] Combining Aggregates

On Thu, Mar 17, 2016 at 10:59 PM, Tomas Vondra wrote: > Hi, > > On 03/17/2016 12:53 PM, David Rowley wrote: >> > ... >> >> >> I just had a quick skim over the patch and noticed the naming >> convention you're using for the combine function is *_pl, and you have >> float8_pl. There's already a func

Re: [HACKERS] Combining Aggregates

On 16 March 2016 at 23:54, Haribabu Kommi wrote: > On Wed, Mar 16, 2016 at 8:34 AM, David Rowley > wrote: >> Yes me too, so I spent several hours yesterday writing all of the >> combine functions and serialisation/deserialisation that are required >> for all of SUM(), AVG() STDDEV*(). I also noti

Re: [HACKERS] Combining Aggregates

On Wed, Mar 16, 2016 at 8:34 AM, David Rowley wrote: > On 16 March 2016 at 06:39, Tomas Vondra wrote: >> After looking at the parallel aggregate patch, I also looked at this one, as >> it's naturally related. Sadly I haven't found any issue that I could nag >> about ;-) The patch seems well baked

Re: [HACKERS] Combining Aggregates

On 16 March 2016 at 06:39, Tomas Vondra wrote: > After looking at the parallel aggregate patch, I also looked at this one, as > it's naturally related. Sadly I haven't found any issue that I could nag > about ;-) The patch seems well baked, as it was in the oven for quite a long > time already. T

Re: [HACKERS] Combining Aggregates

Hi, On 03/14/2016 05:45 AM, David Rowley wrote: On 14 March 2016 at 15:20, David Rowley wrote: Current patch: I've now updated the patch to base it on top of the parallel aggregate patch in [2]. To apply the attached, you must apply [2] first! ... [2] http://www.postgresql.org/message-id/c

Re: [HACKERS] Combining Aggregates

On 1/20/16 11:42 PM, David Rowley wrote: > On 21 January 2016 at 08:06, Robert Haas > wrote: > > On Wed, Jan 20, 2016 at 7:38 AM, David Rowley > mailto:david.row...@2ndquadrant.com>> > wrote: > > Agreed. So I've attached a version of the patch which d

Re: [HACKERS] Combining Aggregates

On Thu, Jan 21, 2016 at 3:42 PM, David Rowley wrote: > On 21 January 2016 at 08:06, Robert Haas wrote: >> >> On Wed, Jan 20, 2016 at 7:38 AM, David Rowley >> wrote: >> > Agreed. So I've attached a version of the patch which does not have any >> > of >> > the serialise/deserialise stuff in it. >>

Re: [HACKERS] Combining Aggregates

On Sat, Jan 23, 2016 at 6:26 PM, Jeff Janes wrote: > On Fri, Jan 22, 2016 at 4:53 PM, David Rowley > wrote: >> >> It seems that I must have mistakenly believed that non-existing >> columns for previous versions were handled using the power of magic. >> Turns out that I was wrong, and they need to

Re: [HACKERS] Combining Aggregates

On Fri, Jan 22, 2016 at 4:53 PM, David Rowley wrote: > > It seems that I must have mistakenly believed that non-existing > columns for previous versions were handled using the power of magic. > Turns out that I was wrong, and they need to be included as dummy > columns in the queries for previous

Re: [HACKERS] Combining Aggregates

On 23 January 2016 at 09:44, David Rowley wrote: > On 23 January 2016 at 09:17, Jeff Janes wrote: >> On Wed, Jan 20, 2016 at 11:06 AM, Robert Haas wrote: >>> On Wed, Jan 20, 2016 at 7:38 AM, David Rowley >>> wrote: Agreed. So I've attached a version of the patch which does not have any of

Re: [HACKERS] Combining Aggregates

On 23 January 2016 at 09:17, Jeff Janes wrote: > On Wed, Jan 20, 2016 at 11:06 AM, Robert Haas wrote: >> On Wed, Jan 20, 2016 at 7:38 AM, David Rowley >> wrote: >>> Agreed. So I've attached a version of the patch which does not have any of >>> the serialise/deserialise stuff in it. >> >> I re-re

Re: [HACKERS] Combining Aggregates

On Wed, Jan 20, 2016 at 11:06 AM, Robert Haas wrote: > On Wed, Jan 20, 2016 at 7:38 AM, David Rowley > wrote: >> Agreed. So I've attached a version of the patch which does not have any of >> the serialise/deserialise stuff in it. > > I re-reviewed this and have committed most of it with only mino

Re: [HACKERS] Combining Aggregates

On Thu, Jan 21, 2016 at 4:08 PM, David Rowley wrote: > It's quite simple to test how much of a win it'll be in the serial > case today, and yes, it's not much, but it's a bit. > > create table t1 as select x from generate_series(1,100) x(x); > vacuum analyze t1; > select count(*) from (select

Re: [HACKERS] Combining Aggregates

On Thu, Jan 21, 2016 at 3:52 PM, David Rowley wrote: > On 21 January 2016 at 15:53, Haribabu Kommi > wrote: >> >> On Thu, Jan 21, 2016 at 1:33 PM, Haribabu Kommi >> wrote: >> > >> > Here I attached updated patch of parallel aggregate based on the latest >> > changes in master. Still it lack of c

Re: [HACKERS] Combining Aggregates

On 22 January 2016 at 06:56, Robert Haas wrote: > On Wed, Jan 20, 2016 at 8:32 PM, David Rowley > wrote: >> The other two usages which I have thought of are; >> >> 1) Aggregating before UNION ALL, which might be fairly simple after the >> grouping planner changes, as it may just be a matter of co

Re: [HACKERS] Combining Aggregates

On Wed, Jan 20, 2016 at 9:33 PM, Haribabu Kommi wrote: > Here I attached updated patch of parallel aggregate based on the latest > changes in master. Still it lack of cost comparison of normal aggregate to > parallel aggregate because of difficulty. This cost comparison is required > in parallel a

Re: [HACKERS] Combining Aggregates

On Wed, Jan 20, 2016 at 8:32 PM, David Rowley wrote: > At the moment I think everything which will use this is queued up behind the > pathification of the grouping planner which Tom is working on. I think > naturally Parallel Aggregate makes sense to work on first, given all the > other parallel s

Re: [HACKERS] Combining Aggregates

On 21 January 2016 at 15:53, Haribabu Kommi wrote: > On Thu, Jan 21, 2016 at 1:33 PM, Haribabu Kommi > wrote: > > > > Here I attached updated patch of parallel aggregate based on the latest > > changes in master. Still it lack of cost comparison of normal aggregate > to > > parallel aggregate be

Re: [HACKERS] Combining Aggregates

On Thu, Jan 21, 2016 at 1:33 PM, Haribabu Kommi wrote: > > Here I attached updated patch of parallel aggregate based on the latest > changes in master. Still it lack of cost comparison of normal aggregate to > parallel aggregate because of difficulty. This cost comparison is required > in parallel

Re: [HACKERS] Combining Aggregates

On Thu, Jan 21, 2016 at 12:32 PM, David Rowley wrote: > On 21 January 2016 at 04:59, Robert Haas wrote: >> >> On Wed, Jan 20, 2016 at 7:53 AM, David Rowley >> wrote: >> > On 21 January 2016 at 01:44, Robert Haas wrote: >> >> >> >> On Wed, Jan 20, 2016 at 7:38 AM, David Rowley >> >> wrote: >> >

Re: [HACKERS] Combining Aggregates

On 21 January 2016 at 08:06, Robert Haas wrote: > On Wed, Jan 20, 2016 at 7:38 AM, David Rowley > wrote: > > Agreed. So I've attached a version of the patch which does not have any > of > > the serialise/deserialise stuff in it. > > I re-reviewed this and have committed most of it with only mino

Re: [HACKERS] Combining Aggregates

On 21 January 2016 at 04:59, Robert Haas wrote: > On Wed, Jan 20, 2016 at 7:53 AM, David Rowley > wrote: > > On 21 January 2016 at 01:44, Robert Haas wrote: > >> > >> On Wed, Jan 20, 2016 at 7:38 AM, David Rowley > >> wrote: > >> >> To my mind, priority #1 ought to be putting this fine new > >

Re: [HACKERS] Combining Aggregates

On Wed, Jan 20, 2016 at 7:38 AM, David Rowley wrote: > Agreed. So I've attached a version of the patch which does not have any of > the serialise/deserialise stuff in it. I re-reviewed this and have committed most of it with only minor kibitizing. A few notes: - I changed the EXPLAIN code, sinc

Re: [HACKERS] Combining Aggregates

On Wed, Jan 20, 2016 at 7:53 AM, David Rowley wrote: > On 21 January 2016 at 01:44, Robert Haas wrote: >> >> On Wed, Jan 20, 2016 at 7:38 AM, David Rowley >> wrote: >> >> To my mind, priority #1 ought to be putting this fine new >> >> functionality to some use. Expanding it to every aggregate w

Re: [HACKERS] Combining Aggregates

On 21 January 2016 at 01:44, Robert Haas wrote: > On Wed, Jan 20, 2016 at 7:38 AM, David Rowley > wrote: > >> To my mind, priority #1 ought to be putting this fine new > >> functionality to some use. Expanding it to every aggregate we've got > >> seems like a distinctly second priority. That's

Re: [HACKERS] Combining Aggregates

On Wed, Jan 20, 2016 at 7:38 AM, David Rowley wrote: >> To my mind, priority #1 ought to be putting this fine new >> functionality to some use. Expanding it to every aggregate we've got >> seems like a distinctly second priority. That's not to say that it's >> absolutely gotta go down that way,

Re: [HACKERS] Combining Aggregates

On 20 January 2016 at 10:54, Robert Haas wrote: > On Tue, Jan 19, 2016 at 4:50 PM, David Rowley > wrote: > >> Oh, one more point: is there any reason why all of this needs to be a > >> single (giant) patch? I feel like the handling of internal states > >> could be a separate patch from the basi

Re: [HACKERS] Combining Aggregates

On Tue, Jan 19, 2016 at 4:50 PM, David Rowley wrote: >> Oh, one more point: is there any reason why all of this needs to be a >> single (giant) patch? I feel like the handling of internal states >> could be a separate patch from the basic patch to allow partial >> aggregation and aggregate-combin

Re: [HACKERS] Combining Aggregates

On 20 January 2016 at 05:58, Robert Haas wrote: > > On Mon, Dec 21, 2015 at 4:02 AM, David Rowley > > wrote: > >> Now, there has been talk of this previously, on various threads, but I > don't > >> believe any final decisions were made on how exactly it should be done. > At > >> the moment I pla

Re: [HACKERS] Combining Aggregates

On 20 January 2016 at 05:56, Robert Haas wrote: > > On Mon, Dec 21, 2015 at 4:02 AM, David Rowley > wrote: > > Now, there has been talk of this previously, on various threads, but I > don't > > believe any final decisions were made on how exactly it should be done. > At > > the moment I plan to

Re: [HACKERS] Combining Aggregates

On Tue, Jan 19, 2016 at 11:56 AM, Robert Haas wrote: > [ rewinding to here from the detour I led us on ] > > On Mon, Dec 21, 2015 at 4:02 AM, David Rowley > wrote: >> Now, there has been talk of this previously, on various threads, but I don't >> believe any final decisions were made on how exact

Re: [HACKERS] Combining Aggregates

[ rewinding to here from the detour I led us on ] On Mon, Dec 21, 2015 at 4:02 AM, David Rowley wrote: > Now, there has been talk of this previously, on various threads, but I don't > believe any final decisions were made on how exactly it should be done. At > the moment I plan to make changes as

Re: [HACKERS] Combining Aggregates

On Tue, Jan 19, 2016 at 12:22 AM, Tomas Vondra wrote: > I dare to claim that if expanded objects require a separate memory context > per aggregate state (I don't see why would be the case), it's a dead end. > Not so long ago we've fixed exactly this issue in array_agg(), which > addressed a bunch

Re: [HACKERS] Combining Aggregates

On Mon, Jan 18, 2016 at 11:24 PM, Tom Lane wrote: > Robert Haas writes: >> Here is a patch that helps a good deal. I changed things so that when >> we create a context, we always allocate at least 1kB. > > That's going to kill performance in some other cases; subtransactions > in particular rely

Re: [HACKERS] Combining Aggregates

On 19 January 2016 at 17:14, Robert Haas wrote: > On Mon, Jan 18, 2016 at 12:34 PM, Robert Haas > wrote: > > On Mon, Jan 18, 2016 at 12:09 PM, Tom Lane wrote: > >> Robert Haas writes: > >>> Yeah. The API contract for an expanded object took me quite a while > >>> to puzzle out, but it seems t

Re: [HACKERS] Combining Aggregates

On 01/19/2016 05:14 AM, Robert Haas wrote: On Mon, Jan 18, 2016 at 12:34 PM, Robert Haas wrote: On Mon, Jan 18, 2016 at 12:09 PM, Tom Lane wrote: Robert Haas writes: Yeah. The API contract for an expanded object took me quite a while to puzzle out, but it seems to be this: if somebody han

Re: [HACKERS] Combining Aggregates

On 19 January 2016 at 18:04, Tomas Vondra wrote: > Hi, > > On 01/19/2016 05:00 AM, David Rowley wrote: > >> On 19 January 2016 at 06:03, Pavel Stehule > > wrote: >> >> ... > >> >> It is strange, why hashaggregate is too slow? >> >> >> Good question. I looked at

Re: [HACKERS] Combining Aggregates

On 01/19/2016 06:04 AM, Tomas Vondra wrote: Hi, On 01/19/2016 05:00 AM, David Rowley wrote: Good question. I looked at this and found my VM was swapping like crazy. Upon investigation it appears that's because, since the patch creates a memory context per aggregated group, and in this case I'

Re: [HACKERS] Combining Aggregates

Hi, On 01/19/2016 05:00 AM, David Rowley wrote: On 19 January 2016 at 06:03, Pavel Stehule mailto:pavel.steh...@gmail.com>> wrote: ... It is strange, why hashaggregate is too slow? Good question. I looked at this and found my VM was swapping like crazy. Upon investigation it appears th

Re: [HACKERS] Combining Aggregates

Robert Haas writes: > Here is a patch that helps a good deal. I changed things so that when > we create a context, we always allocate at least 1kB. That's going to kill performance in some other cases; subtransactions in particular rely on the subtransaction's TransactionContext not causing any

Re: [HACKERS] Combining Aggregates

On Mon, Jan 18, 2016 at 12:34 PM, Robert Haas wrote: > On Mon, Jan 18, 2016 at 12:09 PM, Tom Lane wrote: >> Robert Haas writes: >>> Yeah. The API contract for an expanded object took me quite a while >>> to puzzle out, but it seems to be this: if somebody hands you an R/W >>> pointer to an expa

Re: [HACKERS] Combining Aggregates

On 19 January 2016 at 06:03, Pavel Stehule wrote: > > > > >> > # explain analyze select a%100,length(string_agg(b,',')) from ab >> group >> > by 1; >> > QUERY PLAN >> > >> -

Re: [HACKERS] Combining Aggregates

On 19 January 2016 at 02:44, Haribabu Kommi wrote: > On Mon, Jan 18, 2016 at 10:32 PM, David Rowley > wrote: > > I just thought like direct mapping of the structure with text pointer. > something like > the below. > > result = PG_ARGISNULL(0) ? NULL : (text *) PG_GETARG_POINTER(0); > state = (Po

Re: [HACKERS] Combining Aggregates

On Mon, Jan 18, 2016 at 12:09 PM, Tom Lane wrote: > Robert Haas writes: >> Yeah. The API contract for an expanded object took me quite a while >> to puzzle out, but it seems to be this: if somebody hands you an R/W >> pointer to an expanded object, you're entitled to assume that you can >> "take

Re: [HACKERS] Combining Aggregates

Robert Haas writes: > Yeah. The API contract for an expanded object took me quite a while > to puzzle out, but it seems to be this: if somebody hands you an R/W > pointer to an expanded object, you're entitled to assume that you can > "take over" that object and mutate it however you like. But t

Re: [HACKERS] Combining Aggregates

> > > # explain analyze select a%100,length(string_agg(b,',')) from ab > group > > by 1; > > QUERY PLAN > > > --- > >

Re: [HACKERS] Combining Aggregates

On Mon, Jan 18, 2016 at 6:32 AM, David Rowley wrote: >> In my tests, this seems to be slightly slower than what we're doing >> today; worst of all, it adds a handful of cycles to >> advance_transition_function() even when the aggregate is not an >> expanded object or, indeed, not even pass-by-refe

Re: [HACKERS] Combining Aggregates

On Mon, Jan 18, 2016 at 10:32 PM, David Rowley wrote: > On 18 January 2016 at 16:44, Robert Haas wrote: >> >> On Sun, Jan 17, 2016 at 9:26 PM, David Rowley >> wrote: >> > hmm, so wouldn't that mean that the transition function would need to >> > (for >> > each input tuple): >> > >> > 1. Parse th

Re: [HACKERS] Combining Aggregates

On 18 January 2016 at 16:44, Robert Haas wrote: > On Sun, Jan 17, 2016 at 9:26 PM, David Rowley > wrote: > > hmm, so wouldn't that mean that the transition function would need to > (for > > each input tuple): > > > > 1. Parse that StringInfo into tokens. > > 2. Create a new aggregate state objec

Re: [HACKERS] Combining Aggregates

On Sun, Jan 17, 2016 at 9:26 PM, David Rowley wrote: > hmm, so wouldn't that mean that the transition function would need to (for > each input tuple): > > 1. Parse that StringInfo into tokens. > 2. Create a new aggregate state object. > 3. Populate the new aggregate state based on the tokenised St

Re: [HACKERS] Combining Aggregates

On 18 January 2016 at 14:36, Haribabu Kommi wrote: > On Sat, Jan 16, 2016 at 12:00 PM, David Rowley > wrote: > > On 16 January 2016 at 03:03, Robert Haas wrote: > >> > >> On Tue, Dec 29, 2015 at 7:39 PM, David Rowley > >> wrote: > >> >> No, the idea I had in mind was to allow it to continue to

Re: [HACKERS] Combining Aggregates

On Sat, Jan 16, 2016 at 12:00 PM, David Rowley wrote: > On 16 January 2016 at 03:03, Robert Haas wrote: >> >> On Tue, Dec 29, 2015 at 7:39 PM, David Rowley >> wrote: >> >> No, the idea I had in mind was to allow it to continue to exist in the >> >> expanded format until you really need it in the

Re: [HACKERS] Combining Aggregates

On 16 January 2016 at 03:03, Robert Haas wrote: > On Tue, Dec 29, 2015 at 7:39 PM, David Rowley > wrote: > >> No, the idea I had in mind was to allow it to continue to exist in the > >> expanded format until you really need it in the varlena format, and > >> then serialize it at that point. You

Re: [HACKERS] Combining Aggregates

Man, I really shouldn't go on vacation for Christmas or, like, ever. My email responses get way too slow. Sorry. On Tue, Dec 29, 2015 at 7:39 PM, David Rowley wrote: >> No, the idea I had in mind was to allow it to continue to exist in the >> expanded format until you really need it in the varle

Re: [HACKERS] Combining Aggregates

On Fri, Jan 15, 2016 at 3:34 PM, David Rowley wrote: > On 8 January 2016 at 22:43, David Rowley > wrote: >> >> I've attached some re-based patched on current master. This is just to fix >> a duplicate OID problem. >> > > I've attached two updated patched to fix a conflict with a recent change to

Re: [HACKERS] Combining Aggregates

On 25 December 2015 at 14:10, Robert Haas wrote: > On Mon, Dec 21, 2015 at 4:53 PM, David Rowley > wrote: > > On 22 December 2015 at 01:30, Robert Haas wrote: > >> Can we use Tom's expanded-object stuff instead of introducing > >> aggserialfn and aggdeserialfn? In other words, if you have a >

Re: [HACKERS] Combining Aggregates

On Mon, Dec 21, 2015 at 4:53 PM, David Rowley wrote: > On 22 December 2015 at 01:30, Robert Haas wrote: >> Can we use Tom's expanded-object stuff instead of introducing >> aggserialfn and aggdeserialfn? In other words, if you have a >> aggtranstype = INTERNAL, then what we do is: >> >> 1. Create

Re: [HACKERS] Combining Aggregates

On Thu, Dec 24, 2015 at 1:12 PM, David Rowley wrote: > On 24 December 2015 at 13:55, Haribabu Kommi > wrote: >> >> On Wed, Dec 23, 2015 at 7:50 PM, David Rowley >> wrote: >> > One other part that I'm not too sure on how to deal with is how to set >> > the >> > data type for the Aggrefs when we'r

Re: [HACKERS] Combining Aggregates

On 24 December 2015 at 13:55, Haribabu Kommi wrote: > On Wed, Dec 23, 2015 at 7:50 PM, David Rowley > wrote: > > One other part that I'm not too sure on how to deal with is how to set > the > > data type for the Aggrefs when we're not performing finalization on the > > aggregate node. The return

Re: [HACKERS] Combining Aggregates

On 24 December 2015 at 14:07, Haribabu Kommi wrote: > On Wed, Dec 23, 2015 at 7:50 PM, David Rowley > wrote: > > This is just my serial format that I've come up with for NumericAggState, > > which is basically: "N sumX maxScale maxScaleCount NaNcount". Perhaps we > can > > come up with something

Re: [HACKERS] Combining Aggregates

On 23 December 2015 at 21:50, David Rowley wrote: > Apart from the problems I listed above, I'm reasonably happy with the > patch now, and I'm ready for someone else to take a serious look at it. > I forgot to mention another situation where this patch might need a bit more thought. This does n

Re: [HACKERS] Combining Aggregates

On Wed, Dec 23, 2015 at 7:50 PM, David Rowley wrote: > This is just my serial format that I've come up with for NumericAggState, > which is basically: "N sumX maxScale maxScaleCount NaNcount". Perhaps we can > come up with something better, maybe just packing the ints and int64s into a > bytea typ

Re: [HACKERS] Combining Aggregates

On Wed, Dec 23, 2015 at 7:50 PM, David Rowley wrote: > One other part that I'm not too sure on how to deal with is how to set the > data type for the Aggrefs when we're not performing finalization on the > aggregate node. The return type for the Aggref in this case will be either > the transtype,

Re: [HACKERS] Combining Aggregates

On 22 December 2015 at 01:30, Robert Haas wrote: > Can we use Tom's expanded-object stuff instead of introducing > aggserialfn and aggdeserialfn? In other words, if you have a > aggtranstype = INTERNAL, then what we do is: > > 1. Create a new data type that represents the transition state. > 2.

Re: [HACKERS] Combining Aggregates

On Mon, Dec 21, 2015 at 4:02 AM, David Rowley wrote: > Ok, so it seems that my mindset was not in parallel process space when I > was thinking about this. I think having the pointer in the Tuple is > probably going to be fine for this multiple stage aggregation when that's > occurring in a singl

Re: [HACKERS] Combining Aggregates

On 18 December 2015 at 01:28, David Rowley wrote: > # select sum(x::numeric) from generate_series(1,3) x(x); > ERROR: invalid memory alloc request size 18446744072025250716 > > The reason that this happens is down to the executor always thinking that > Aggref returns the aggtype, but in cases wh

Re: [HACKERS] Combining Aggregates

On Thu, Dec 3, 2015 at 12:01 AM, David Rowley wrote: > On 27 July 2015 at 04:58, Heikki Linnakangas wrote: >> This patch seems sane to me, as far as it goes. However, there's no >> planner or executor code to use the aggregate combining for anything. I'm >> not a big fan of dead code, I'd really

  1   2   >