Re: [HACKERS] multiset patch review

2011-02-16 Thread Robert Haas
On Tue, Feb 15, 2011 at 7:13 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Feb 15, 2011 at 4:31 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: array_flatten() no longer exists. I added array_trim() as an alias to trim_array() because it would be a FAQ. I don't like the alias

Re: [HACKERS] multiset patch review

2011-02-15 Thread Robert Haas
On Tue, Feb 15, 2011 at 4:31 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: array_flatten() no longer exists. I added array_trim() as an alias to trim_array() because it would be a FAQ. I don't like the alias thing - let's add one name or the other, not both. Similarly, let's NOT add

Re: [HACKERS] multiset patch review

2011-02-12 Thread Stephen Frost
Itagaki, * Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: On Sat, Feb 12, 2011 at 05:01, Stephen Frost sfr...@snowman.net wrote: What does the spec say about this, if anything?  Is that required by spec, or is the spec not relevant to this because MULTISETs are only one

Re: [HACKERS] multiset patch review

2011-02-11 Thread Robert Haas
On Fri, Feb 4, 2011 at 9:11 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Sat, Feb 5, 2011 at 04:24, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: In math class, maybe.  But in programming, no.  Multiset is a datatype.  Array is a different datatype.  There is no reason why we

Re: [HACKERS] multiset patch review

2011-02-11 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Right, but making the parser slower has a cost, too. ScanKeywordLookup() is already a hotspot in some workloads, and there's overhead buried in the bison parser, too. Yeah. Keep in mind that a bison parser fundamentally runs off a two-dimensional

Re: [HACKERS] multiset patch review

2011-02-11 Thread Itagaki Takahiro
On Sat, Feb 12, 2011 at 00:50, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Right, but making the parser slower has a cost, too. ScanKeywordLookup() is already a hotspot in some workloads, and there's overhead buried in the bison parser, too. Yeah.  Keep in

Re: [HACKERS] multiset patch review

2011-02-11 Thread Robert Haas
On Fri, Feb 11, 2011 at 11:17 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: Did you measure the actual cost in the real world? If we are using such a sensitive parser, it should be a problem even without the patch. It *is* a problem without the patch! Adding unnecessary keywords is

Re: [HACKERS] multiset patch review

2011-02-11 Thread Stephen Frost
* Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: I will remove parser changes from the patch; it will add only a few array functions. Then, please let me know functions you don't want to include in the core, if any. I'll remove them at the same time. Seems like this should be 'waiting on

Re: [HACKERS] multiset patch review

2011-02-11 Thread Itagaki Takahiro
On Sat, Feb 12, 2011 at 05:01, Stephen Frost sfr...@snowman.net wrote: Input arrays are always flattened into one-dimensional arrays. That just strikes me as completely broken when it comes to PG Arrays. Contains operators (@, , @) ignore multi-dimensions. Array slice operator ([lo:hi]) always

Re: [HACKERS] multiset patch review

2011-02-04 Thread Robert Haas
On Mon, Jan 31, 2011 at 1:49 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: I removed collect() aggregate function because the result type is MULTISET in the spec. I keep all of other functions and operators because they won't break anything in the standard. When we will have true

Re: [HACKERS] multiset patch review

2011-02-04 Thread Itagaki Takahiro
On Sat, Feb 5, 2011 at 02:29, Robert Haas robertmh...@gmail.com wrote: I am still not in favor of adding this syntax. I also don't like the syntax, but unfortunately, almost all of them are in the SQL standard :-(. In addition, Oracle already uses the same feature with the special syntax,

Re: [HACKERS] multiset patch review

2011-02-04 Thread Robert Haas
On Fri, Feb 4, 2011 at 1:02 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Sat, Feb 5, 2011 at 02:29, Robert Haas robertmh...@gmail.com wrote: I am still not in favor of adding this syntax. I also don't like the syntax, but unfortunately, almost all of them are in the SQL standard

Re: [HACKERS] multiset patch review

2011-02-04 Thread Itagaki Takahiro
On Sat, Feb 5, 2011 at 03:04, Robert Haas robertmh...@gmail.com wrote: I am still not in favor of adding this syntax. I also don't like the syntax, but unfortunately, almost all of them are in the SQL standard :-(. The standard specifies this syntax for arrays, or for multisets? Multisets.

Re: [HACKERS] multiset patch review

2011-02-04 Thread Robert Haas
On Fri, Feb 4, 2011 at 1:15 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Sat, Feb 5, 2011 at 03:04, Robert Haas robertmh...@gmail.com wrote: I am still not in favor of adding this syntax. I also don't like the syntax, but unfortunately, almost all of them are in the SQL standard

Re: [HACKERS] multiset patch review

2011-02-04 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: On Fri, Feb 4, 2011 at 1:15 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: Multisets. But I chose the same function name and syntax because arrays *are* multisets by definition. In math class, maybe. But in programming, no. Multiset is a

Re: [HACKERS] multiset patch review

2011-02-04 Thread Itagaki Takahiro
On Sat, Feb 5, 2011 at 04:24, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: In math class, maybe.  But in programming, no.  Multiset is a datatype.  Array is a different datatype.  There is no reason why we need to clutter our parser with extra keywords to support a non-standard feature

Re: [HACKERS] multiset patch review

2011-01-30 Thread Robert Haas
On Mon, Jan 24, 2011 at 7:27 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Mon, Jan 24, 2011 at 20:49, Robert Haas robertmh...@gmail.com wrote: I notice that this is adding keywords and syntax support for what is basically a PostgreSQL extension (since we certainly can't possibly be

Re: [HACKERS] multiset patch review

2011-01-30 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: So, the plan is to add this now with non-standard semantics and then change the semantics later if and when we implement what the standard requires? That's not something we usually do, and I don't see why it's a better idea in this case than it is in

Re: [HACKERS] multiset patch review

2011-01-30 Thread Pavel Stehule
2011/1/30 Robert Haas robertmh...@gmail.com: On Mon, Jan 24, 2011 at 7:27 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Mon, Jan 24, 2011 at 20:49, Robert Haas robertmh...@gmail.com wrote: I notice that this is adding keywords and syntax support for what is basically a PostgreSQL

Re: [HACKERS] multiset patch review

2011-01-30 Thread Robert Haas
On Sun, Jan 30, 2011 at 12:16 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: So, the plan is to add this now with non-standard semantics and then change the semantics later if and when we implement what the standard requires?  That's not something we usually

Re: [HACKERS] multiset patch review

2011-01-30 Thread Itagaki Takahiro
On Mon, Jan 31, 2011 at 02:34, Robert Haas robertmh...@gmail.com wrote: I'm in favor of rejecting this patch in its entirety.  The functionality looks useful, but once you remove the syntax support, it could just as easily be distributed as a contrib module rather than in core. +1 ... if

Re: [HACKERS] multiset patch review

2011-01-30 Thread Robert Haas
On Sun, Jan 30, 2011 at 1:46 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Mon, Jan 31, 2011 at 02:34, Robert Haas robertmh...@gmail.com wrote: I'm in favor of rejecting this patch in its entirety.  The functionality looks useful, but once you remove the syntax support, it could

Re: [HACKERS] multiset patch review

2011-01-30 Thread Robert Haas
On Sun, Jan 30, 2011 at 2:11 PM, Robert Haas robertmh...@gmail.com wrote: On Sun, Jan 30, 2011 at 1:46 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Mon, Jan 31, 2011 at 02:34, Robert Haas robertmh...@gmail.com wrote: I'm in favor of rejecting this patch in its entirety.  The

Re: [HACKERS] multiset patch review

2011-01-30 Thread Itagaki Takahiro
On Mon, Jan 31, 2011 at 04:12, Robert Haas robertmh...@gmail.com wrote: Well, do you want to revise this and submit a stripped-down version? I'm not averse to adding things that are required by the standard and won't cause backward compatibility problems later. Sure. I'll remove collect()

Re: [HACKERS] multiset patch review

2011-01-24 Thread Robert Haas
On Mon, Jan 24, 2011 at 2:45 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: [ latest patch ] I notice that this is adding keywords and syntax support for what is basically a PostgreSQL extension (since we certainly can't possibly be following the SQL standards given that we're not

Re: [HACKERS] multiset patch review

2011-01-24 Thread Itagaki Takahiro
On Mon, Jan 24, 2011 at 20:49, Robert Haas robertmh...@gmail.com wrote: I notice that this is adding keywords and syntax support for what is basically a PostgreSQL extension (since we certainly can't possibly be following the SQL standards given that we're not implementing a new datatype.  Is

Re: [HACKERS] multiset patch review

2011-01-12 Thread Itagaki Takahiro
On Wed, Jan 12, 2011 at 15:21, Peter Eisentraut pete...@gmx.net wrote: You may want to read this thread about the cardinality function are you trying to add: http://archives.postgresql.org/pgsql-hackers/2009-02/msg01388.php Since our archive is split per month, this might be more readable:

Re: [HACKERS] multiset patch review

2011-01-12 Thread Pavel Stehule
Hello there is one issue - probably useless checking a type equality in function check_comparable and check_concatinatable, because when your function is registrated with arguments (anyarray, anyarray), then is guaranteed so type of array1 is same as type of array2, and then you don't need to

Re: [HACKERS] multiset patch review

2011-01-12 Thread Itagaki Takahiro
On Wed, Jan 12, 2011 at 20:18, Pavel Stehule pavel.steh...@gmail.com wrote: there is one issue - probably useless checking a type equality in function check_comparable and check_concatinatable, because when your function is registrated with arguments (anyarray, anyarray), then is guaranteed so

Re: [HACKERS] multiset patch review

2011-01-12 Thread Pavel Stehule
2011/1/12 Itagaki Takahiro itagaki.takah...@gmail.com: On Wed, Jan 12, 2011 at 20:18, Pavel Stehule pavel.steh...@gmail.com wrote: there is one issue - probably useless checking a type equality in function check_comparable and check_concatinatable, because when your function is registrated

Re: [HACKERS] multiset patch review

2011-01-12 Thread Pavel Stehule
2011/1/12 Pavel Stehule pavel.steh...@gmail.com: 2011/1/12 Itagaki Takahiro itagaki.takah...@gmail.com: On Wed, Jan 12, 2011 at 20:18, Pavel Stehule pavel.steh...@gmail.com wrote: there is one issue - probably useless checking a type equality in function check_comparable and

Re: [HACKERS] multiset patch review

2011-01-12 Thread Alvaro Herrera
Excerpts from Itagaki Takahiro's message of mié ene 12 01:52:12 -0300 2011: Separate patches for src and doc attached. It includes a few bug fixes and cleanup. I changed the error code in trim_array() to ERRCODE_ARRAY_ELEMENT_ERROR according to the spec. Two small nitpicks: + static void +

Re: [HACKERS] multiset patch review

2011-01-12 Thread Itagaki Takahiro
On Wed, Jan 12, 2011 at 23:29, Alvaro Herrera alvhe...@commandprompt.com wrote: Two small nitpicks: + check_concatinatable(Oid element_type1, Oid element_type2) +         ereport(ERROR, +                 (errcode(ERRCODE_DATATYPE_MISMATCH), +                  errmsg(cannot concatenate

Re: [HACKERS] multiset patch review

2011-01-11 Thread Peter Eisentraut
On ons, 2011-01-12 at 13:52 +0900, Itagaki Takahiro wrote: I added a short description about MULTISET and example of operators in Arrays 8.14.7. Multiset Support section in the docs. Is it enough? or what kind of information do you want? Separate patches for src and doc attached. It

Re: [HACKERS] MULTISET patch

2011-01-06 Thread Erik Rijkers
On Thu, January 6, 2011 12:54, Itagaki Takahiro wrote: Here is an updated patch for MULTISET functions support. There seem to be some faulty line-endings in arrays.out that break the arrays test (on x86_64 Centos 5.4). make, make check were OK after I removed these (3 lines, from line 1370).

Re: [HACKERS] MULTISET patch

2010-12-27 Thread Simon Riggs
On Mon, 2010-12-27 at 10:36 +0900, Itagaki Takahiro wrote: On Mon, Dec 27, 2010 at 02:09, Pavel Stehule pavel.steh...@gmail.com wrote: I have a free time and I can do a review of your patch. Please, can send a last version and can send a links on documentation that you used? Thanks! The

Re: [HACKERS] MULTISET patch

2010-12-27 Thread Pavel Stehule
Hello some quick notes: * trim_array - you use a deconstruct_array. It unpack all fields and it could not be effective. Can we limit a unpacked array? I searched on net. This function has a little bit unconptual name - DB2 use a synonym for this function array_trim. Can we use this synonym

Re: [HACKERS] MULTISET patch

2010-12-27 Thread Itagaki Takahiro
On Mon, Dec 27, 2010 at 20:15, Pavel Stehule pavel.steh...@gmail.com wrote: * trim_array - you use a deconstruct_array. It unpack  all fields and it could not be effective. Can we limit a unpacked array? Sure, I'll optimize it. I searched on net. This function has a little bit unconptual name

Re: [HACKERS] MULTISET patch

2010-12-27 Thread Pavel Stehule
2010/12/27 Itagaki Takahiro itagaki.takah...@gmail.com: On Mon, Dec 27, 2010 at 20:15, Pavel Stehule pavel.steh...@gmail.com wrote: * trim_array - you use a deconstruct_array. It unpack  all fields and it could not be effective. Can we limit a unpacked array? Sure, I'll optimize it. I