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] 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]: https://postgr.es/m/caj7c6to3x88dgd8c4tb-eq2zdpz+9mp+kowdzk_82bez_cm...@mail.gmail.com -- Best regards, Aleksander Alekseev
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
