[PATCH] Rename trim_array() for consistency with the rest of array_* functions
Hi hackers,
Currently most of the functions dealing with arrays ( except for
unnest() and cardinality() ) start with `array_` prefix [1]https://www.postgresql.org/docs/current/functions-array.html -
array_length(), array_fill(), etc. Also this is how we name the new
functions, e.g. recently proposed array_sort() [2]https://commitfest.postgresql.org/50/5277/ and array_reverse()
[3]: https://commitfest.postgresql.org/50/5314/
somewhat misleading. For instance, it is not show in the output of:
```
\df array_*
```
The proposed patch renames trim_array() to array_trim(). The old
function is kept for backward compatibility but is marked as
deprecated and is left undocumented. It can be removed in 10 years
from now or so. To my knowledge this is how we typically deprecate old
functions and operators.
Thoughts?
[1]: https://www.postgresql.org/docs/current/functions-array.html
[2]: https://commitfest.postgresql.org/50/5277/
[3]: https://commitfest.postgresql.org/50/5314/
--
Best regards,
Aleksander Alekseev
Attachments:
v1-0001-Rename-trim_array-to-array_trim.patchapplication/octet-stream; name=v1-0001-Rename-trim_array-to-array_trim.patchDownload
From e89be4c69d134645903a77ba83d41ed1b9c77658 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Tue, 29 Oct 2024 14:32:59 +0300
Subject: [PATCH v1] Rename trim_array() to array_trim()
The new name is more consistent with the rest of array_* functions.
trim_array() is kept for backward compatibility but is left undocumented.
It can be removed completely in the far future.
Aleksander Alekseev, reviewed by TODO FIXME
Discussion: TODO FIXME
BUMP CATVERSION
---
doc/src/sgml/func.sgml | 6 +++---
src/backend/utils/adt/arrayfuncs.c | 2 +-
src/include/catalog/pg_proc.dat | 7 +++++--
src/test/regress/expected/arrays.out | 12 ++++++------
src/test/regress/sql/arrays.sql | 10 +++++-----
5 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 05f630c6a6..9a6dd4d776 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20482,9 +20482,9 @@ SELECT NULLIF(value, '(none)') ...
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
- <primary>trim_array</primary>
+ <primary>array_trim</primary>
</indexterm>
- <function>trim_array</function> ( <parameter>array</parameter> <type>anyarray</type>, <parameter>n</parameter> <type>integer</type> )
+ <function>array_trim</function> ( <parameter>array</parameter> <type>anyarray</type>, <parameter>n</parameter> <type>integer</type> )
<returnvalue>anyarray</returnvalue>
</para>
<para>
@@ -20492,7 +20492,7 @@ SELECT NULLIF(value, '(none)') ...
If the array is multidimensional, only the first dimension is trimmed.
</para>
<para>
- <literal>trim_array(ARRAY[1,2,3,4,5,6], 2)</literal>
+ <literal>array_trim(ARRAY[1,2,3,4,5,6], 2)</literal>
<returnvalue>{1,2,3,4}</returnvalue>
</para></entry>
</row>
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index a715e7e0b8..f967bc81ea 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -6917,7 +6917,7 @@ width_bucket_array_variable(Datum operand,
* Only the first dimension is trimmed.
*/
Datum
-trim_array(PG_FUNCTION_ARGS)
+array_trim(PG_FUNCTION_ARGS)
{
ArrayType *v = PG_GETARG_ARRAYTYPE_P(0);
int n = PG_GETARG_INT32(1);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 1ec0d6f6b5..6f5b7c6a22 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -1732,9 +1732,12 @@
proname => 'width_bucket', prorettype => 'int4',
proargtypes => 'anycompatible anycompatiblearray',
prosrc => 'width_bucket_array' },
-{ oid => '6172', descr => 'remove last N elements of array',
+{ oid => '6172', descr => 'deprecated; an alias for array_trim',
proname => 'trim_array', prorettype => 'anyarray',
- proargtypes => 'anyarray int4', prosrc => 'trim_array' },
+ proargtypes => 'anyarray int4', prosrc => 'array_trim' },
+{ oid => '8140', descr => 'remove last N elements of array',
+ proname => 'array_trim', prorettype => 'anyarray',
+ proargtypes => 'anyarray int4', prosrc => 'array_trim' },
{ oid => '6215', descr => 'shuffle array',
proname => 'array_shuffle', provolatile => 'v', prorettype => 'anyarray',
proargtypes => 'anyarray', prosrc => 'array_shuffle' },
diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out
index a6d81fd5f9..d819393d65 100644
--- a/src/test/regress/expected/arrays.out
+++ b/src/test/regress/expected/arrays.out
@@ -2626,15 +2626,15 @@ SELECT width_bucket(5, ARRAY[3, 4, NULL]);
ERROR: thresholds array must not contain NULLs
SELECT width_bucket(5, ARRAY[ARRAY[1, 2], ARRAY[3, 4]]);
ERROR: thresholds must be one-dimensional array
--- trim_array
-SELECT arr, trim_array(arr, 2)
+-- array_trim
+SELECT arr, array_trim(arr, 2)
FROM
(VALUES ('{1,2,3,4,5,6}'::bigint[]),
('{1,2}'),
('[10:16]={1,2,3,4,5,6,7}'),
('[-15:-10]={1,2,3,4,5,6}'),
('{{1,10},{2,20},{3,30},{4,40}}')) v(arr);
- arr | trim_array
+ arr | array_trim
-------------------------------+-----------------
{1,2,3,4,5,6} | {1,2,3,4}
{1,2} | {}
@@ -2643,11 +2643,11 @@ FROM
{{1,10},{2,20},{3,30},{4,40}} | {{1,10},{2,20}}
(5 rows)
-SELECT trim_array(ARRAY[1, 2, 3], -1); -- fail
+SELECT array_trim(ARRAY[1, 2, 3], -1); -- fail
ERROR: number of elements to trim must be between 0 and 3
-SELECT trim_array(ARRAY[1, 2, 3], 10); -- fail
+SELECT array_trim(ARRAY[1, 2, 3], 10); -- fail
ERROR: number of elements to trim must be between 0 and 3
-SELECT trim_array(ARRAY[]::int[], 1); -- fail
+SELECT array_trim(ARRAY[]::int[], 1); -- fail
ERROR: number of elements to trim must be between 0 and 0
-- array_shuffle
SELECT array_shuffle('{1,2,3,4,5,6}'::int[]) <@ '{1,2,3,4,5,6}';
diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql
index 47058dfde5..04af09f422 100644
--- a/src/test/regress/sql/arrays.sql
+++ b/src/test/regress/sql/arrays.sql
@@ -800,9 +800,9 @@ SELECT width_bucket('5'::text, ARRAY[3, 4]::integer[]);
SELECT width_bucket(5, ARRAY[3, 4, NULL]);
SELECT width_bucket(5, ARRAY[ARRAY[1, 2], ARRAY[3, 4]]);
--- trim_array
+-- array_trim
-SELECT arr, trim_array(arr, 2)
+SELECT arr, array_trim(arr, 2)
FROM
(VALUES ('{1,2,3,4,5,6}'::bigint[]),
('{1,2}'),
@@ -810,9 +810,9 @@ FROM
('[-15:-10]={1,2,3,4,5,6}'),
('{{1,10},{2,20},{3,30},{4,40}}')) v(arr);
-SELECT trim_array(ARRAY[1, 2, 3], -1); -- fail
-SELECT trim_array(ARRAY[1, 2, 3], 10); -- fail
-SELECT trim_array(ARRAY[]::int[], 1); -- fail
+SELECT array_trim(ARRAY[1, 2, 3], -1); -- fail
+SELECT array_trim(ARRAY[1, 2, 3], 10); -- fail
+SELECT array_trim(ARRAY[]::int[], 1); -- fail
-- array_shuffle
SELECT array_shuffle('{1,2,3,4,5,6}'::int[]) <@ '{1,2,3,4,5,6}';
--
2.47.0
On 29/10/2024 13:01, Aleksander Alekseev wrote:
Hi hackers,
Currently most of the functions dealing with arrays ( except for
unnest() and cardinality() ) start with `array_` prefix [1] -
array_length(), array_fill(), etc. Also this is how we name the new
functions, e.g. recently proposed array_sort() [2] and array_reverse()
[3]. The only exception from this rule is trim_array() which might be
somewhat misleading. For instance, it is not show in the output of:```
\df array_*
```The proposed patch renames trim_array() to array_trim(). The old
function is kept for backward compatibility but is marked as
deprecated and is left undocumented. It can be removed in 10 years
from now or so. To my knowledge this is how we typically deprecate old
functions and operators.Thoughts?
While unfortunately named, we cannot deprecate TRIM_ARRAY() because it
is mandated by the standard.
--
Vik Fearing
Em ter., 29 de out. de 2024 às 09:01, Aleksander Alekseev <
aleksander@timescale.com> escreveu:
The proposed patch renames trim_array() to array_trim().
That Array Functions list on func.sgml is an ordered list, so the right
place for it should be before array_upper, right ?
regards
Marcos
Hi Vik,
While unfortunately named, we cannot deprecate TRIM_ARRAY() because it
is mandated by the standard.
I didn't know that, thanks.
Still PostgreSQL doesn't follow the standard precisely in more than
one respect. Perhaps we should add array_trim() and keep both? Or
(less likely) remove trim_array() and document this deviation from the
standard? Or at least document why it's named this way.
What do you think?
--
Best regards,
Aleksander Alekseev
On Tue, Oct 29, 2024 at 03:23:15PM +0300, Aleksander Alekseev wrote:
While unfortunately named, we cannot deprecate TRIM_ARRAY() because it
is mandated by the standard.I didn't know that, thanks.
Wow. Neither did I.
Still PostgreSQL doesn't follow the standard precisely in more than
one respect. Perhaps we should add array_trim() and keep both? Or
(less likely) remove trim_array() and document this deviation from the
standard? Or at least document why it's named this way.
I suspect that it is not the only function in this case, and that we
don't document that it is in the standard. I would suggest to let
things the way they are on HEAD and withdraw the patch.
--
Michael
Hi Michael,
On Tue, Oct 29, 2024 at 03:23:15PM +0300, Aleksander Alekseev wrote:
While unfortunately named, we cannot deprecate TRIM_ARRAY() because it
is mandated by the standard.I didn't know that, thanks.
Wow. Neither did I.
Still PostgreSQL doesn't follow the standard precisely in more than
one respect. Perhaps we should add array_trim() and keep both? Or
(less likely) remove trim_array() and document this deviation from the
standard? Or at least document why it's named this way.I suspect that it is not the only function in this case, and that we
don't document that it is in the standard. I would suggest to let
things the way they are on HEAD and withdraw the patch.
Thanks for your feedback. The patch is withdrawn.
--
Best regards,
Aleksander Alekseev