Re: [HACKERS] Support for array_remove and array_replace functions

2012-07-11 Thread Tom Lane
Marco Nenciarini marco.nenciar...@2ndquadrant.it writes:
 Patch v3 attached.

I'm looking at this patch now.  The restriction of array_remove to
one-dimensional arrays seems a bit annoying.  I see the difficulty:
if the input is multi-dimensional then removing some elements could
lead to a non-rectangular array, which isn't supported.  However,
that could be dealt with by decreeing that the *result* is
one-dimensional and of the necessary length, regardless of the
dimensionality of the input.

I'm not actually certain whether that's a better definition or not.
But one less error case seems like generally a good thing.
Comments?

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Support for array_remove and array_replace functions

2012-07-11 Thread Alex Hunsaker
On Wed, Jul 11, 2012 at 9:54 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Marco Nenciarini marco.nenciar...@2ndquadrant.it writes:
 Patch v3 attached.

 I'm looking at this patch now.  The restriction of array_remove to
 one-dimensional arrays seems a bit annoying.  I see the difficulty:
 if the input is multi-dimensional then removing some elements could
 lead to a non-rectangular array, which isn't supported.  However,
 that could be dealt with by decreeing that the *result* is
 one-dimensional and of the necessary length, regardless of the
 dimensionality of the input.

Makes sense to me. +1

The other option ISTM is to replace removed entries with NULL-- which
I don't really like.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Support for array_remove and array_replace functions

2012-07-11 Thread Tom Lane
Alex Hunsaker bada...@gmail.com writes:
 On Wed, Jul 11, 2012 at 9:54 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I'm looking at this patch now.  The restriction of array_remove to
 one-dimensional arrays seems a bit annoying.  I see the difficulty:
 if the input is multi-dimensional then removing some elements could
 lead to a non-rectangular array, which isn't supported.  However,
 that could be dealt with by decreeing that the *result* is
 one-dimensional and of the necessary length, regardless of the
 dimensionality of the input.

 Makes sense to me. +1

 The other option ISTM is to replace removed entries with NULL-- which
 I don't really like.

Well, you can do that with array_replace, so I don't see a need to
define array_remove that way.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Support for array_remove and array_replace functions

2012-07-11 Thread Robert Haas
On Jul 11, 2012, at 11:53 AM, Alex Hunsaker bada...@gmail.com wrote:
 On Wed, Jul 11, 2012 at 9:54 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Marco Nenciarini marco.nenciar...@2ndquadrant.it writes:
 Patch v3 attached.
 
 I'm looking at this patch now.  The restriction of array_remove to
 one-dimensional arrays seems a bit annoying.  I see the difficulty:
 if the input is multi-dimensional then removing some elements could
 lead to a non-rectangular array, which isn't supported.  However,
 that could be dealt with by decreeing that the *result* is
 one-dimensional and of the necessary length, regardless of the
 dimensionality of the input.
 
 Makes sense to me. +1

+1 from me, too.

...Robert

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Support for array_remove and array_replace functions

2012-07-11 Thread Tom Lane
Marco Nenciarini marco.nenciar...@2ndquadrant.it writes:
 Patch v3 attached.

Applied with mostly-but-not-entirely cosmetic adjustments.

I left array_remove throwing error for multi-dimensional arrays for
the moment, because I realized that changing the dimensionality as
I suggested would conflict with the optimization to return the original
array if there were no matches.  I don't think we'd want the definition
to read multidimensional arrays are changed to one dimension, but only
if at least one element is removed --- that's getting a little too
weird.  If anyone's really hot to make it work on multi-D arrays, we
could consider disabling that optimization; it's not clear to me that
it's worth a lot.  But for now I'm willing to stick with the
throw-an-error approach.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Support for array_remove and array_replace functions

2012-07-01 Thread Alex Hunsaker
On Sat, Jun 30, 2012 at 3:28 PM, Marco Nenciarini
marco.nenciar...@2ndquadrant.it wrote:

 On 30/06/2012 04:16, Alex Hunsaker wrote:
 
  Hi, I've been reviewing this patch.
 
  Good documentation, and regression tests. The code looked fine but I
  didn't care for the code duplication between array_replace and
  array_remove so I merged those into a helper function,
  array_replace_internal(). Thoughts?

 It looks reasonable.

 There was a typo in array_replace which was caught by regression tests.
 I've fixed the typo and changed a comment in array_replace_internal.

 Patch v3 attached.

Looks good to me, marked ready for commiter.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Support for array_remove and array_replace functions

2012-06-30 Thread Marco Nenciarini
On 30/06/2012 04:16, Alex Hunsaker wrote:
  
 Hi, I've been reviewing this patch.
 
 Good documentation, and regression tests. The code looked fine but I
 didn't care for the code duplication between array_replace and
 array_remove so I merged those into a helper function,
 array_replace_internal(). Thoughts?

It looks reasonable.

There was a typo in array_replace which was caught by regression tests.
I've fixed the typo and changed a comment in array_replace_internal.

Patch v3 attached.

Regards,
Marco

-- 
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it


array-functions-v3.patch.bz2
Description: application/bzip

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Support for array_remove and array_replace functions

2012-06-29 Thread Alex Hunsaker
On Thu, Jun 14, 2012 at 4:41 AM, Marco Nenciarini 
marco.nenciar...@2ndquadrant.it wrote:

 Hi,

  following Gabriele's email regarding our previous patch on Foreign
 Key Arrays[1], I am sending a subset of that patch which includes only
 two array functions which will be needed in that patch: array_remove
 (limited to single-dimensional arrays) and array_replace.



  The patch includes changes to the documentation.


Hi, I've been reviewing this patch.

Good documentation, and regression tests. The code looked fine but I didn't
care for the code duplication between array_replace and array_remove so I
merged those into a helper function, array_replace_internal(). Thoughts?

Other than that it all looks good to me.


array-functions_v2.patch.bz2
Description: BZip2 compressed data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Support for array_remove and array_replace functions

2012-06-14 Thread Marco Nenciarini
Hi,

  following Gabriele's email regarding our previous patch on Foreign
Key Arrays[1], I am sending a subset of that patch which includes only
two array functions which will be needed in that patch: array_remove
(limited to single-dimensional arrays) and array_replace.

  The patch includes changes to the documentation.

Cheers,
Marco

[1] http://archives.postgresql.org/message-id/4FD8F422.40709%402ndQuadrant.it

-- 
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it 



array-functions.patch.bz2
Description: application/bzip

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers