Re: [HACKERS] Function array_agg(array)

2014-11-25 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes: 2014-10-27 11:20 GMT+01:00 Ali Akbar the.ap...@gmail.com: [ array_agg_anyarray-13.patch ] This patch is ready for commit I've committed this after some significant modifications. I did not like the API chosen, specifically the business about

Re: [HACKERS] Function array_agg(array)

2014-11-25 Thread Ali Akbar
2014-11-26 0:38 GMT+07:00 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: 2014-10-27 11:20 GMT+01:00 Ali Akbar the.ap...@gmail.com: [ array_agg_anyarray-13.patch ] This patch is ready for commit I've committed this after some significant modifications. I did

Re: [HACKERS] Function array_agg(array)

2014-11-25 Thread Tom Lane
Ali Akbar the.ap...@gmail.com writes: Just curious, in accumArrayResultArr, while enlarging array and nullbitmaps, why it's implemented with: astate-abytes = Max(astate-abytes * 2, astate-nbytes + ndatabytes); and astate-aitems = Max(astate-aitems * 2, newnitems); won't it be more

Re: [HACKERS] Function array_agg(array)

2014-10-27 Thread Ali Akbar
2014-10-27 9:11 GMT+07:00 Ali Akbar the.ap...@gmail.com: 2014-10-27 1:38 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: Hi My idea is using new ArrayBuilder optimized for building multidimensional arrays with own State type. I think so casting to ArrayBuildState is base of our problems,

Re: [HACKERS] Function array_agg(array)

2014-10-27 Thread Pavel Stehule
Hi I did some minor changes in code * move tests of old or new builder style for array sublink out of main cycles * some API simplification of new builder - we should not to create identical API, mainly it has no sense Regards Pavel Stehule 2014-10-27 8:12 GMT+01:00 Ali Akbar

Re: [HACKERS] Function array_agg(array)

2014-10-27 Thread Ali Akbar
2014-10-27 16:15 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: Hi I did some minor changes in code * move tests of old or new builder style for array sublink out of main cycles * some API simplification of new builder - we should not to create identical API, mainly it has no sense

Re: [HACKERS] Function array_agg(array)

2014-10-27 Thread Pavel Stehule
2014-10-27 11:20 GMT+01:00 Ali Akbar the.ap...@gmail.com: 2014-10-27 16:15 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: Hi I did some minor changes in code * move tests of old or new builder style for array sublink out of main cycles * some API simplification of new builder - we

Re: [HACKERS] Function array_agg(array)

2014-10-27 Thread Ali Akbar
super I tested last version and I have not any objections. 1. We would to have this feature - it is long time number of our ToDo List 2. Proposal and design of multidimensional aggregation is clean and nobody has objection here. 3. There is zero impact on current implementation. From

Re: [HACKERS] Function array_agg(array)

2014-10-26 Thread Pavel Stehule
Hi My idea is using new ArrayBuilder optimized for building multidimensional arrays with own State type. I think so casting to ArrayBuildState is base of our problems, so I don't would to do. Code in array_agg_* is simple, little bit more complex code is in nodeSubplan.c. Some schematic changes

Re: [HACKERS] Function array_agg(array)

2014-10-26 Thread Ali Akbar
2014-10-27 1:38 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: Hi My idea is using new ArrayBuilder optimized for building multidimensional arrays with own State type. I think so casting to ArrayBuildState is base of our problems, so I don't would to do. Code in array_agg_* is simple,

Re: [HACKERS] Function array_agg(array)

2014-10-25 Thread Ali Akbar
2014-10-25 10:29 GMT+07:00 Ali Akbar the.ap...@gmail.com: I fixed small issue in regress tests and I enhanced tests for varlena types and null values. Thanks. it is about 15% faster than original implementation. 15% faster than array_agg(scalar)? I haven't verify the performance, but

Re: [HACKERS] Function array_agg(array)

2014-10-25 Thread Pavel Stehule
2014-10-25 8:19 GMT+02:00 Ali Akbar the.ap...@gmail.com: 2014-10-25 10:29 GMT+07:00 Ali Akbar the.ap...@gmail.com: I fixed small issue in regress tests and I enhanced tests for varlena types and null values. Thanks. it is about 15% faster than original implementation. 15% faster than

Re: [HACKERS] Function array_agg(array)

2014-10-25 Thread Pavel Stehule
2014-10-25 8:33 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: 2014-10-25 8:19 GMT+02:00 Ali Akbar the.ap...@gmail.com: 2014-10-25 10:29 GMT+07:00 Ali Akbar the.ap...@gmail.com: I fixed small issue in regress tests and I enhanced tests for varlena types and null values. Thanks. it

Re: [HACKERS] Function array_agg(array)

2014-10-25 Thread Ali Akbar
makeArrayResult1 - I have no better name now I found one next minor detail. you reuse a array_agg_transfn function. Inside is a message array_agg_transfn called in non-aggregate context. It is not correct for array_agg_anyarray_transfn Fixed. probably specification dim and lbs in

Re: [HACKERS] Function array_agg(array)

2014-10-25 Thread Pavel Stehule
2014-10-25 10:16 GMT+02:00 Ali Akbar the.ap...@gmail.com: makeArrayResult1 - I have no better name now I found one next minor detail. you reuse a array_agg_transfn function. Inside is a message array_agg_transfn called in non-aggregate context. It is not correct for

Re: [HACKERS] Function array_agg(array)

2014-10-25 Thread Ali Akbar
2014-10-25 15:43 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: 2014-10-25 10:16 GMT+02:00 Ali Akbar the.ap...@gmail.com: makeArrayResult1 - I have no better name now I found one next minor detail. you reuse a array_agg_transfn function. Inside is a message array_agg_transfn called

Re: [HACKERS] Function array_agg(array)

2014-10-25 Thread Pavel Stehule
2014-10-25 12:20 GMT+02:00 Ali Akbar the.ap...@gmail.com: 2014-10-25 15:43 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: 2014-10-25 10:16 GMT+02:00 Ali Akbar the.ap...@gmail.com: makeArrayResult1 - I have no better name now I found one next minor detail. you reuse a

Re: [HACKERS] Function array_agg(array)

2014-10-25 Thread Ali Akbar
you can check it? We can test, how performance lost we get. As second benefit we can get numbers for introduction new optimized array builder array_agg(anyarray) with deconstruct_array, unchanged accumArrayResult and makeMdArrayResult: INSERT 0 1 Time: 852,527 ms INSERT 0 1 Time: 844,275

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Ali Akbar
Updated patch attached. 2014-10-22 20:51 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: I agree with your proposal. I have a few comments to design: 1. patch doesn't hold documentation and regress tests, please append it. i've added some regression tests in arrays.sql and aggregate.sql.

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Pavel Stehule
Hi it looks well doc: http://www.postgresql.org/docs/9.4/static/sql-expressions.html#SQL-SYNTAX-ARRAY-CONSTRUCTORS it should be fixed too Regards Pavel 2014-10-24 10:24 GMT+02:00 Ali Akbar the.ap...@gmail.com: Updated patch attached. 2014-10-22 20:51 GMT+07:00 Pavel Stehule

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Ali Akbar
2014-10-24 15:48 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: Hi it looks well doc: http://www.postgresql.org/docs/9.4/static/sql-expressions.html#SQL-SYNTAX-ARRAY-CONSTRUCTORS it should be fixed too Regards Pavel doc updated with additional example for array(subselect). patch

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Pavel Stehule
Hi some in last patch is wrong, I cannot to compile it: arrayfuncs.c: In function ‘accumArrayResult’: arrayfuncs.c:4603:9: error: ‘ArrayBuildState’ has no member named ‘alen’ astate-alen = 64; /* arbitrary starting array size */ ^ arrayfuncs.c:4604:9: error: ‘ArrayBuildState’ has no

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Ali Akbar
2014-10-24 16:26 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: Hi some in last patch is wrong, I cannot to compile it: arrayfuncs.c: In function ‘accumArrayResult’: arrayfuncs.c:4603:9: error: ‘ArrayBuildState’ has no member named ‘alen’ astate-alen = 64; /* arbitrary starting array

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Michael Paquier
On Fri, Oct 24, 2014 at 11:43 AM, Ali Akbar the.ap...@gmail.com wrote: 2014-10-24 16:26 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: Hi some in last patch is wrong, I cannot to compile it: arrayfuncs.c: In function 'accumArrayResult': arrayfuncs.c:4603:9: error: 'ArrayBuildState' has

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Pavel Stehule
2014-10-24 11:43 GMT+02:00 Ali Akbar the.ap...@gmail.com: 2014-10-24 16:26 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: Hi some in last patch is wrong, I cannot to compile it: arrayfuncs.c: In function ‘accumArrayResult’: arrayfuncs.c:4603:9: error: ‘ArrayBuildState’ has no member

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Pavel Stehule
Hi I did some performance tests and it is interesting: it is about 15% faster than original implementation. Regards Pavel 2014-10-24 13:58 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: 2014-10-24 11:43 GMT+02:00 Ali Akbar the.ap...@gmail.com: 2014-10-24 16:26 GMT+07:00 Pavel

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Alvaro Herrera
Michael Paquier wrote: That's not surprising, sometimes filterdiff misses the shot. Really? Wow, that's bad news. I've been using it to submit patches from time to time ... -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Pavel Stehule
2014-10-24 13:58 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: 2014-10-24 11:43 GMT+02:00 Ali Akbar the.ap...@gmail.com: 2014-10-24 16:26 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: Hi some in last patch is wrong, I cannot to compile it: arrayfuncs.c: In function

Re: [HACKERS] Function array_agg(array)

2014-10-24 Thread Pavel Stehule
Hi Ali I checked a code. I am thinking so code organization is not good. accumArrayResult is too long now. makeMdArrayResult will not work, when arrays was joined (it is not consistent now). I don't like a usage of state-is_array_accum in array_userfunc.c -- it is signal of wrong wrapping. next

Re: [HACKERS] Function array_agg(array)

2014-10-22 Thread Pavel Stehule
Hi 2014-10-19 8:02 GMT+02:00 Ali Akbar the.ap...@gmail.com: So, is there any idea how we will handle NULL and empty array in array_agg(anyarray)? I propose we just reject those input because the output will make no sense: - array_agg(NULL::int[]) -- the result will be indistinguished from

Re: [HACKERS] Function array_agg(array)

2014-10-22 Thread Ali Akbar
Thanks for the review 2014-10-22 20:51 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: I agree with your proposal. I have a few comments to design: 1. patch doesn't hold documentation and regress tests, please append it. OK, i'll add the documentation and regression test 2. this

Re: [HACKERS] Function array_agg(array)

2014-10-22 Thread Pavel Stehule
2014-10-22 16:58 GMT+02:00 Ali Akbar the.ap...@gmail.com: Thanks for the review 2014-10-22 20:51 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: I agree with your proposal. I have a few comments to design: 1. patch doesn't hold documentation and regress tests, please append it. OK,

Re: [HACKERS] Function array_agg(array)

2014-10-22 Thread Ali Akbar
2014-10-22 22:48 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: 2014-10-22 16:58 GMT+02:00 Ali Akbar the.ap...@gmail.com: Thanks for the review 2014-10-22 20:51 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: I agree with your proposal. I have a few comments to design: 1. patch

Re: [HACKERS] Function array_agg(array)

2014-10-22 Thread Pavel Stehule
2014-10-23 4:00 GMT+02:00 Ali Akbar the.ap...@gmail.com: 2014-10-22 22:48 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: 2014-10-22 16:58 GMT+02:00 Ali Akbar the.ap...@gmail.com: Thanks for the review 2014-10-22 20:51 GMT+07:00 Pavel Stehule pavel.steh...@gmail.com: I agree with your

Re: [HACKERS] Function array_agg(array)

2014-10-19 Thread Ali Akbar
So, is there any idea how we will handle NULL and empty array in array_agg(anyarray)? I propose we just reject those input because the output will make no sense: - array_agg(NULL::int[]) -- the result will be indistinguished from array_agg of NULL ints. - array_agg('{}'::int[]) -- how we

Re: [HACKERS] Function array_agg(array)

2014-10-12 Thread Ali Akbar
2014-10-11 22:28 GMT+07:00 Tom Lane t...@sss.pgh.pa.us: Seems dangerous as heck; certainly it would have side-effects far more wide-ranging than just making this particular function work. A safer answer is to split array_agg into two functions, array_agg(anynonarray) - anyarray

[HACKERS] Function array_agg(array)

2014-10-11 Thread Ali Akbar
Greetings, While looking for easier items in PostgreSQL Wiki's Todo List (this will be my 3rd patch), i found this TODO: Add a built-in array_agg(anyarray) or similar, that can aggregate 1-dimensional arrays into a 2-dimensional array. I've stumbled by this lack of array_agg(anyarray)

Re: [HACKERS] Function array_agg(array)

2014-10-11 Thread Tom Lane
Ali Akbar the.ap...@gmail.com writes: So, can't we just set the typarray of array types to its self oid? Seems dangerous as heck; certainly it would have side-effects far more wide-ranging than just making this particular function work. A safer answer is to split array_agg into two functions,