[PATCH] Add array_reverse() function
Hi,
Recently I wanted to call array_reverse() and discovered that we still
don't have it. I'm not the first one who encountered this limitation.
array_reverse() was requested at least since 2009 [1]/messages/by-id/4AEE80FC.7010608@postnewspapers.com.au and the
workaround on PostgreSQL Wiki is dated 2010 [2]https://wiki.postgresql.org/wiki/Array_reverse.
The proposed patch adds this function. Only the first dimension of the
array is reversed, for consistency with the existing functions such as
array_shuffle() [3]https://www.postgresql.org/docs/current/functions-array.html.
Examples:
=# SELECT array_reverse(ARRAY[1,2,3,NULL]);
array_reverse
---------------
{NULL,3,2,1}
=# SELECT array_reverse(ARRAY[[1,2],[3,4],[5,6]]);
array_reverse
---------------------
{{5,6},{3,4},{1,2}}
Thoughts?
[1]: /messages/by-id/4AEE80FC.7010608@postnewspapers.com.au
[2]: https://wiki.postgresql.org/wiki/Array_reverse
[3]: https://www.postgresql.org/docs/current/functions-array.html
--
Best regards,
Aleksander Alekseev
Attachments:
v1-0001-Add-array_reverse-function.patchapplication/octet-stream; name=v1-0001-Add-array_reverse-function.patchDownload
From e9515105182d78156e4f3b0b6802547f03e08055 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Mon, 21 Oct 2024 11:22:20 +0300
Subject: [PATCH v1] Add array_reverse() function.
Aleksander Alekseev, reviewed by TODO FIXME
Discussion: TODO FIXME
BUMP CATVERSION
---
doc/src/sgml/func.sgml | 17 ++++
src/backend/utils/adt/array_userfuncs.c | 116 ++++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 3 +
src/test/regress/expected/arrays.out | 31 +++++++
src/test/regress/sql/arrays.sql | 7 ++
5 files changed, 174 insertions(+)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index ad663c94d7..2e9e3f4b9b 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20379,6 +20379,23 @@ SELECT NULLIF(value, '(none)') ...
</para></entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>array_reverse</primary>
+ </indexterm>
+ <function>array_reverse</function> ( <type>anyarray</type> )
+ <returnvalue>anyarray</returnvalue>
+ </para>
+ <para>
+ Reverses the first dimension of the array.
+ </para>
+ <para>
+ <literal>array_reverse(ARRAY[[1,2],[3,4],[5,6]])</literal>
+ <returnvalue>{{5,6},{3,4},{1,2}}</returnvalue>
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c
index 6599be2ec5..8862840368 100644
--- a/src/backend/utils/adt/array_userfuncs.c
+++ b/src/backend/utils/adt/array_userfuncs.c
@@ -1685,3 +1685,119 @@ array_sample(PG_FUNCTION_ARGS)
PG_RETURN_ARRAYTYPE_P(result);
}
+
+
+/*
+ * array_reverse_n
+ * Return a copy of array with reversed items.
+ *
+ * NOTE: it would be cleaner to look up the elmlen/elmbval/elmalign info
+ * from the system catalogs, given only the elmtyp. However, the caller is
+ * in a better position to cache this info across multiple calls.
+ */
+static ArrayType *
+array_reverse_n(ArrayType *array, Oid elmtyp, TypeCacheEntry *typentry)
+{
+ ArrayType *result;
+ int ndim,
+ *dims,
+ *lbs,
+ nelm,
+ nitem,
+ rdims[MAXDIM],
+ rlbs[MAXDIM];
+ int16 elmlen;
+ bool elmbyval;
+ char elmalign;
+ Datum *elms,
+ *ielms;
+ bool *nuls,
+ *inuls;
+
+ ndim = ARR_NDIM(array);
+ dims = ARR_DIMS(array);
+ lbs = ARR_LBOUND(array);
+
+ elmlen = typentry->typlen;
+ elmbyval = typentry->typbyval;
+ elmalign = typentry->typalign;
+
+ /* If the target array is empty, exit fast */
+ if (ndim < 1 || dims[0] < 1)
+ return construct_empty_array(elmtyp);
+
+ deconstruct_array(array, elmtyp, elmlen, elmbyval, elmalign,
+ &elms, &nuls, &nelm);
+
+ nitem = dims[0]; /* total number of items */
+ nelm /= nitem; /* number of elements per item */
+
+ /* Reverse the array */
+ ielms = elms;
+ inuls = nuls;
+ for (int i = 0; i < nitem / 2; i++)
+ {
+ int j = (nitem - i - 1) * nelm;
+ Datum *jelms = elms + j;
+ bool *jnuls = nuls + j;
+
+ /* Swap i'th and j'th items; advance ielms/inuls to next item */
+ for (int k = 0; k < nelm; k++)
+ {
+ Datum elm = *ielms;
+ bool nul = *inuls;
+
+ *ielms++ = *jelms;
+ *inuls++ = *jnuls;
+ *jelms++ = elm;
+ *jnuls++ = nul;
+ }
+ }
+
+ /* Set up dimensions of the result */
+ memcpy(rdims, dims, ndim * sizeof(int));
+ memcpy(rlbs, lbs, ndim * sizeof(int));
+ rdims[0] = nitem;
+
+ result = construct_md_array(elms, nuls, ndim, rdims, rlbs,
+ elmtyp, elmlen, elmbyval, elmalign);
+
+ pfree(elms);
+ pfree(nuls);
+
+ return result;
+}
+
+/*
+ * array_reverse
+ *
+ * Returns an array with the same dimensions as the input array, with its
+ * first-dimension elements in reverse order.
+ */
+Datum
+array_reverse(PG_FUNCTION_ARGS)
+{
+ ArrayType *array = PG_GETARG_ARRAYTYPE_P(0);
+ ArrayType *result;
+ Oid elmtyp;
+ TypeCacheEntry *typentry;
+
+ /*
+ * There is no point in reversing empty arrays or arrays with less than
+ * two items.
+ */
+ if (ARR_NDIM(array) < 1 || ARR_DIMS(array)[0] < 2)
+ PG_RETURN_ARRAYTYPE_P(array);
+
+ elmtyp = ARR_ELEMTYPE(array);
+ typentry = (TypeCacheEntry *) fcinfo->flinfo->fn_extra;
+ if (typentry == NULL || typentry->type_id != elmtyp)
+ {
+ typentry = lookup_type_cache(elmtyp, 0);
+ fcinfo->flinfo->fn_extra = (void *) typentry;
+ }
+
+ result = array_reverse_n(array, elmtyp, typentry);
+
+ PG_RETURN_ARRAYTYPE_P(result);
+}
\ No newline at end of file
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 7c0b74fe05..b805086062 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -1741,6 +1741,9 @@
{ oid => '6216', descr => 'take samples from array',
proname => 'array_sample', provolatile => 'v', prorettype => 'anyarray',
proargtypes => 'anyarray int4', prosrc => 'array_sample' },
+{ oid => '8686', descr => 'reverse array',
+ proname => 'array_reverse', prorettype => 'anyarray',
+ proargtypes => 'anyarray', prosrc => 'array_reverse' },
{ oid => '3816', descr => 'array typanalyze',
proname => 'array_typanalyze', provolatile => 's', prorettype => 'bool',
proargtypes => 'internal', prosrc => 'array_typanalyze' },
diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out
index a6d81fd5f9..0b61fb5bb7 100644
--- a/src/test/regress/expected/arrays.out
+++ b/src/test/regress/expected/arrays.out
@@ -2703,3 +2703,34 @@ SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
ERROR: sample size must be between 0 and 6
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
ERROR: sample size must be between 0 and 6
+-- array_reverse
+SELECT array_reverse('{}'::int[]);
+ array_reverse
+---------------
+ {}
+(1 row)
+
+SELECT array_reverse('{1}'::int[]);
+ array_reverse
+---------------
+ {1}
+(1 row)
+
+SELECT array_reverse('{1,2}'::int[]);
+ array_reverse
+---------------
+ {2,1}
+(1 row)
+
+SELECT array_reverse('{1,2,3,NULL,4,5,6}'::int[]);
+ array_reverse
+--------------------
+ {6,5,4,NULL,3,2,1}
+(1 row)
+
+SELECT array_reverse('{{1,2},{3,4},{5,6},{7,8}}'::int[]);
+ array_reverse
+---------------------------
+ {{7,8},{5,6},{3,4},{1,2}}
+(1 row)
+
diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql
index 47058dfde5..691cff4a12 100644
--- a/src/test/regress/sql/arrays.sql
+++ b/src/test/regress/sql/arrays.sql
@@ -827,3 +827,10 @@ SELECT array_dims(array_sample('[-1:2][2:3]={{1,2},{3,NULL},{5,6},{7,8}}'::int[]
SELECT array_dims(array_sample('{{{1,2},{3,NULL}},{{5,6},{7,8}},{{9,10},{11,12}}}'::int[], 2));
SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
+
+-- array_reverse
+SELECT array_reverse('{}'::int[]);
+SELECT array_reverse('{1}'::int[]);
+SELECT array_reverse('{1,2}'::int[]);
+SELECT array_reverse('{1,2,3,NULL,4,5,6}'::int[]);
+SELECT array_reverse('{{1,2},{3,4},{5,6},{7,8}}'::int[]);
\ No newline at end of file
--
2.47.0
On Mon, Oct 21, 2024 at 2:36 PM Aleksander Alekseev
<aleksander@timescale.com> wrote:
Hi,
Recently I wanted to call array_reverse() and discovered that we still
don't have it. I'm not the first one who encountered this limitation.
array_reverse() was requested at least since 2009 [1] and the
workaround on PostgreSQL Wiki is dated 2010 [2].The proposed patch adds this function. Only the first dimension of the
array is reversed, for consistency with the existing functions such as
array_shuffle() [3].Examples:
=# SELECT array_reverse(ARRAY[1,2,3,NULL]);
array_reverse
---------------
{NULL,3,2,1}=# SELECT array_reverse(ARRAY[[1,2],[3,4],[5,6]]);
array_reverse
---------------------
{{5,6},{3,4},{1,2}}Thoughts?
Looks useful. Glancing quickly at the code I have a comment
+
+ /* If the target array is empty, exit fast */
+ if (ndim < 1 || dims[0] < 1)
+ return construct_empty_array(elmtyp);This is taken care by the caller, at least the ndim < 1 case.
+ /*
+ * There is no point in reversing empty arrays or arrays with less than
+ * two items.
+ */
+ if (ARR_NDIM(array) < 1 || ARR_DIMS(array)[0] < 2)
+ PG_RETURN_ARRAYTYPE_P(array);But it returns the input array as is. I think it should at least make
a new copy of input array.
--
Best Wishes,
Ashutosh Bapat
On Mon, Oct 21, 2024, at 11:06, Aleksander Alekseev wrote:
Hi,
Recently I wanted to call array_reverse() and discovered that we still
don't have it. I'm not the first one who encountered this limitation.
array_reverse() was requested at least since 2009 [1] and the
workaround on PostgreSQL Wiki is dated 2010 [2].The proposed patch adds this function. Only the first dimension of the
array is reversed, for consistency with the existing functions such as
array_shuffle() [3].
+1
I've needed this many times.
/Joel
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:
On Mon, Oct 21, 2024 at 2:36 PM Aleksander Alekseev <aleksander@timescale.com> wrote: + /* + * There is no point in reversing empty arrays or arrays with less than + * two items. + */ + if (ARR_NDIM(array) < 1 || ARR_DIMS(array)[0] < 2) + PG_RETURN_ARRAYTYPE_P(array);
But it returns the input array as is. I think it should at least make
a new copy of input array.
I don't think that's really necessary. We have other functions that
will return an input value unchanged without copying it. A
longstanding example is array_larger. Also, this code looks to be
copied from array_shuffle.
regards, tom lane
Hi everyone,
Thanks for the feedback!
But it returns the input array as is. I think it should at least make
a new copy of input array.I don't think that's really necessary. We have other functions that
will return an input value unchanged without copying it. A
longstanding example is array_larger. Also, this code looks to be
copied from array_shuffle.
It was indeed copied from array_shuffle().
Makes me wonder if we should have an utility function which would
contain common code for array_shuffle() and array_reverse().
Considering inconveniences such an approach caused in case of common
code for text and bytea types [1]/messages/by-id/CAJ7c6TO3X88dGd8C4Tb-Eq2ZDPz+9mP+KOwdzK_82BEz_cMPZg@mail.gmail.com I choose to copy the code and modify
it for the task.
+ + /* If the target array is empty, exit fast */ + if (ndim < 1 || dims[0] < 1) + return construct_empty_array(elmtyp);This is taken care by the caller, at least the ndim < 1 case.
Agree. Here is the corrected patch.
[1]: /messages/by-id/CAJ7c6TO3X88dGd8C4Tb-Eq2ZDPz+9mP+KOwdzK_82BEz_cMPZg@mail.gmail.com
--
Best regards,
Aleksander Alekseev
Attachments:
v2-0001-Add-array_reverse-function.patchapplication/x-patch; name=v2-0001-Add-array_reverse-function.patchDownload
From 71ce762f28cad2a2fb13a44595ffe8346dfb29bc Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Mon, 21 Oct 2024 11:22:20 +0300
Subject: [PATCH v2] Add array_reverse() function.
Only the first dimension of the array is reversed, for consistency with
the existing functions such as array_shuffle().
Aleksander Alekseev, reviewed by Ashutosh Bapat, Tom Lane
Discussion: https://postgr.es/m/CAJ7c6TMpeO_ke+QGOaAx9xdJuxa7r=49-anMh3G5476e3CX1CA@mail.gmail.com
BUMP CATVERSION
---
doc/src/sgml/func.sgml | 17 ++++
src/backend/utils/adt/array_userfuncs.c | 112 ++++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 3 +
src/test/regress/expected/arrays.out | 31 +++++++
src/test/regress/sql/arrays.sql | 7 ++
5 files changed, 170 insertions(+)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index ad663c94d7..2e9e3f4b9b 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20379,6 +20379,23 @@ SELECT NULLIF(value, '(none)') ...
</para></entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>array_reverse</primary>
+ </indexterm>
+ <function>array_reverse</function> ( <type>anyarray</type> )
+ <returnvalue>anyarray</returnvalue>
+ </para>
+ <para>
+ Reverses the first dimension of the array.
+ </para>
+ <para>
+ <literal>array_reverse(ARRAY[[1,2],[3,4],[5,6]])</literal>
+ <returnvalue>{{5,6},{3,4},{1,2}}</returnvalue>
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c
index 6599be2ec5..8eccc9ebde 100644
--- a/src/backend/utils/adt/array_userfuncs.c
+++ b/src/backend/utils/adt/array_userfuncs.c
@@ -1685,3 +1685,115 @@ array_sample(PG_FUNCTION_ARGS)
PG_RETURN_ARRAYTYPE_P(result);
}
+
+
+/*
+ * array_reverse_n
+ * Return a copy of array with reversed items.
+ *
+ * NOTE: it would be cleaner to look up the elmlen/elmbval/elmalign info
+ * from the system catalogs, given only the elmtyp. However, the caller is
+ * in a better position to cache this info across multiple calls.
+ */
+static ArrayType *
+array_reverse_n(ArrayType *array, Oid elmtyp, TypeCacheEntry *typentry)
+{
+ ArrayType *result;
+ int ndim,
+ *dims,
+ *lbs,
+ nelm,
+ nitem,
+ rdims[MAXDIM],
+ rlbs[MAXDIM];
+ int16 elmlen;
+ bool elmbyval;
+ char elmalign;
+ Datum *elms,
+ *ielms;
+ bool *nuls,
+ *inuls;
+
+ ndim = ARR_NDIM(array);
+ dims = ARR_DIMS(array);
+ lbs = ARR_LBOUND(array);
+
+ elmlen = typentry->typlen;
+ elmbyval = typentry->typbyval;
+ elmalign = typentry->typalign;
+
+ deconstruct_array(array, elmtyp, elmlen, elmbyval, elmalign,
+ &elms, &nuls, &nelm);
+
+ nitem = dims[0]; /* total number of items */
+ nelm /= nitem; /* number of elements per item */
+
+ /* Reverse the array */
+ ielms = elms;
+ inuls = nuls;
+ for (int i = 0; i < nitem / 2; i++)
+ {
+ int j = (nitem - i - 1) * nelm;
+ Datum *jelms = elms + j;
+ bool *jnuls = nuls + j;
+
+ /* Swap i'th and j'th items; advance ielms/inuls to next item */
+ for (int k = 0; k < nelm; k++)
+ {
+ Datum elm = *ielms;
+ bool nul = *inuls;
+
+ *ielms++ = *jelms;
+ *inuls++ = *jnuls;
+ *jelms++ = elm;
+ *jnuls++ = nul;
+ }
+ }
+
+ /* Set up dimensions of the result */
+ memcpy(rdims, dims, ndim * sizeof(int));
+ memcpy(rlbs, lbs, ndim * sizeof(int));
+ rdims[0] = nitem;
+
+ result = construct_md_array(elms, nuls, ndim, rdims, rlbs,
+ elmtyp, elmlen, elmbyval, elmalign);
+
+ pfree(elms);
+ pfree(nuls);
+
+ return result;
+}
+
+/*
+ * array_reverse
+ *
+ * Returns an array with the same dimensions as the input array, with its
+ * first-dimension elements in reverse order.
+ */
+Datum
+array_reverse(PG_FUNCTION_ARGS)
+{
+ ArrayType *array = PG_GETARG_ARRAYTYPE_P(0);
+ ArrayType *result;
+ Oid elmtyp;
+ TypeCacheEntry *typentry;
+
+ /*
+ * There is no point in reversing empty arrays or arrays with less than
+ * two items.
+ */
+ if (ARR_NDIM(array) < 1 || ARR_DIMS(array)[0] < 2)
+ PG_RETURN_ARRAYTYPE_P(array);
+
+ elmtyp = ARR_ELEMTYPE(array);
+ typentry = (TypeCacheEntry *) fcinfo->flinfo->fn_extra;
+ if (typentry == NULL || typentry->type_id != elmtyp)
+ {
+ typentry = lookup_type_cache(elmtyp, 0);
+ fcinfo->flinfo->fn_extra = (void *) typentry;
+ }
+
+ result = array_reverse_n(array, elmtyp, typentry);
+
+ PG_RETURN_ARRAYTYPE_P(result);
+}
\ No newline at end of file
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 7c0b74fe05..b805086062 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -1741,6 +1741,9 @@
{ oid => '6216', descr => 'take samples from array',
proname => 'array_sample', provolatile => 'v', prorettype => 'anyarray',
proargtypes => 'anyarray int4', prosrc => 'array_sample' },
+{ oid => '8686', descr => 'reverse array',
+ proname => 'array_reverse', prorettype => 'anyarray',
+ proargtypes => 'anyarray', prosrc => 'array_reverse' },
{ oid => '3816', descr => 'array typanalyze',
proname => 'array_typanalyze', provolatile => 's', prorettype => 'bool',
proargtypes => 'internal', prosrc => 'array_typanalyze' },
diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out
index a6d81fd5f9..0b61fb5bb7 100644
--- a/src/test/regress/expected/arrays.out
+++ b/src/test/regress/expected/arrays.out
@@ -2703,3 +2703,34 @@ SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
ERROR: sample size must be between 0 and 6
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
ERROR: sample size must be between 0 and 6
+-- array_reverse
+SELECT array_reverse('{}'::int[]);
+ array_reverse
+---------------
+ {}
+(1 row)
+
+SELECT array_reverse('{1}'::int[]);
+ array_reverse
+---------------
+ {1}
+(1 row)
+
+SELECT array_reverse('{1,2}'::int[]);
+ array_reverse
+---------------
+ {2,1}
+(1 row)
+
+SELECT array_reverse('{1,2,3,NULL,4,5,6}'::int[]);
+ array_reverse
+--------------------
+ {6,5,4,NULL,3,2,1}
+(1 row)
+
+SELECT array_reverse('{{1,2},{3,4},{5,6},{7,8}}'::int[]);
+ array_reverse
+---------------------------
+ {{7,8},{5,6},{3,4},{1,2}}
+(1 row)
+
diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql
index 47058dfde5..691cff4a12 100644
--- a/src/test/regress/sql/arrays.sql
+++ b/src/test/regress/sql/arrays.sql
@@ -827,3 +827,10 @@ SELECT array_dims(array_sample('[-1:2][2:3]={{1,2},{3,NULL},{5,6},{7,8}}'::int[]
SELECT array_dims(array_sample('{{{1,2},{3,NULL}},{{5,6},{7,8}},{{9,10},{11,12}}}'::int[], 2));
SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
+
+-- array_reverse
+SELECT array_reverse('{}'::int[]);
+SELECT array_reverse('{1}'::int[]);
+SELECT array_reverse('{1,2}'::int[]);
+SELECT array_reverse('{1,2,3,NULL,4,5,6}'::int[]);
+SELECT array_reverse('{{1,2},{3,4},{5,6},{7,8}}'::int[]);
\ No newline at end of file
--
2.47.0
On Tuesday, October 22, 2024 16:27 MSK, Aleksander Alekseev <aleksander@timescale.com> wrote:
Hi everyone,
Thanks for the feedback!
But it returns the input array as is. I think it should at least make
a new copy of input array.I don't think that's really necessary. We have other functions that
will return an input value unchanged without copying it. A
longstanding example is array_larger. Also, this code looks to be
copied from array_shuffle.
It was indeed copied from array_shuffle().
Makes me wonder if we should have an utility function which would
contain common code for array_shuffle() and array_reverse().
Considering inconveniences such an approach caused in case of common
code for text and bytea types [1]/messages/by-id/CAJ7c6TO3X88dGd8C4Tb-Eq2ZDPz+9mP+KOwdzK_82BEz_cMPZg@mail.gmail.com I choose to copy the code and modify
it for the task.
+ + /* If the target array is empty, exit fast */ + if (ndim < 1 || dims[0] < 1) + return construct_empty_array(elmtyp);This is taken care by the caller, at least the ndim < 1 case.
Agree. Here is the corrected patch.
[1]: /messages/by-id/CAJ7c6TO3X88dGd8C4Tb-Eq2ZDPz+9mP+KOwdzK_82BEz_cMPZg@mail.gmail.com
--
Best regards,
Aleksander Alekseev
Hi!
Content.
The proposed function waited long to be implemented and it will be very
useful. Personally I used slow workaround, when I needed the reverse of
the array.
Run.
This patch applies cleanly to HEAD. All regression tests pass
successfully against the patch.
Format.
The code formatted according to The Code Guidelines
Documentation.
The documentation is updated, the description of the function is added.
From my point of view, it would be better to mention, that function returns
updated array (does not updates it in place, as a reader can understand),
but other array returning functions in the documentation has the same
style (silently assume: reverse == return reversed array).
Conclusion.
+1 for commiter review
Best regards,
Vladlen Popolitov
postgrespro.com
--
Best regards,
Vladlen Popolitov.
On Wed, Oct 30, 2024 at 10:11:20PM +0300, Пополитов Владлен wrote:
Makes me wonder if we should have an utility function which would
contain common code for array_shuffle() and array_reverse().
Considering inconveniences such an approach caused in case of common
code for text and bytea types [1] I choose to copy the code and modify
it for the task.
We are talking about 20 lines copied over to reverse the order of
these elements that require their own inner for the element lookups
and manipulations. FWIW, I can live with this burden rather than
invent a routine that encapsulates both as this has also the
disadvantage of hiding more internals across more layers.
The abstraction with array_reverse_n() is unnecessary, but you have an
argument with the consistency and the shuffling inner routine.
Perhaps somebody will reuse that to invent a function that works on a
subset, where the comment on top array_reverse_n() about the type
caching still works. Anyway, no objections to what the patch defines
and does.
So it looks rather OK seen from here, as you are proposing.
--
Michael
The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, failed
Spec compliant: tested, failed
Documentation: tested, failed
Content.
The proposed function waited long to be implemented and it will be very
useful. Personally I used slow workaround, when I needed the reverse of
the array.
Run.
This patch applies cleanly to HEAD. All regression tests pass
successfully against the patch.
Format.
The code formatted according to The Code Guidelines
Documentation.
The documentation is updated, the description of the function is added.
From my point of view, it would be better to mention, that function returns
updated array (does not updates it in place, as a reader can understand),
but other array returning functions in the documentation has the same
style (silently assume: reverse == return reversed array).
Conclusion.
+1 for commiter review