Adding argument names to aggregate functions

Started by Dagfinn Ilmari Mannsåkeralmost 3 years ago15 messages
1 attachment(s)

Hi hackers,

I'm sure I'm not the only one who can never remember which way around
the value and delimiter arguments go for string_agg() and has to look it
up in the manual every time. To make it more convenient, here's a patch
that adds proargnames to its pg_proc entries so that it can be seen with
a quick \df in psql.

I also added names to json(b)_object_agg() for good measure, even though
they're more obvious. The remaining built-in multi-argument aggregate
functions are the stats-related ones, where it's all just Y/X (but why
in that order?), so I didn't think it was necessary. If others feel more
strongly, I can add those too.

- ilmari

Attachments:

0001-Add-argument-names-to-multi-argument-aggregates.patchtext/x-diffDownload
From 73f323d5e97dca2e2452f5be199864a8358559c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 27 Feb 2023 13:06:29 +0000
Subject: [PATCH] Add argument names to multi-argument aggregates

This makes it easier to see which way around the arguments go when
using \dfa.  This is particularly relevant for string_agg(), but add
it to json(b)_object_agg() too for good measure.
---
 src/include/catalog/pg_proc.dat | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index e2a7642a2b..f96d29278f 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4988,6 +4988,7 @@
 { oid => '3538', descr => 'concatenate aggregate input into a string',
   proname => 'string_agg', prokind => 'a', proisstrict => 'f',
   prorettype => 'text', proargtypes => 'text text',
+  proargnames => '{value,delimiter}',
   prosrc => 'aggregate_dummy' },
 { oid => '3543', descr => 'aggregate transition function',
   proname => 'bytea_string_agg_transfn', proisstrict => 'f',
@@ -5000,6 +5001,7 @@
 { oid => '3545', descr => 'concatenate aggregate input into a bytea',
   proname => 'string_agg', prokind => 'a', proisstrict => 'f',
   prorettype => 'bytea', proargtypes => 'bytea bytea',
+  proargnames => '{value,delimiter}',
   prosrc => 'aggregate_dummy' },
 
 # To ASCII conversion
@@ -8899,6 +8901,7 @@
 { oid => '3197', descr => 'aggregate input into a json object',
   proname => 'json_object_agg', prokind => 'a', proisstrict => 'f',
   provolatile => 's', prorettype => 'json', proargtypes => 'any any',
+  proargnames => '{key,value}',
   prosrc => 'aggregate_dummy' },
 { oid => '3198', descr => 'build a json array from any inputs',
   proname => 'json_build_array', provariadic => 'any', proisstrict => 'f',
@@ -9791,6 +9794,7 @@
 { oid => '3270', descr => 'aggregate inputs into jsonb object',
   proname => 'jsonb_object_agg', prokind => 'a', proisstrict => 'f',
   prorettype => 'jsonb', proargtypes => 'any any',
+  proargnames => '{key,value}',
   prosrc => 'aggregate_dummy' },
 { oid => '3271', descr => 'build a jsonb array from any inputs',
   proname => 'jsonb_build_array', provariadic => 'any', proisstrict => 'f',
-- 
2.39.1

#2Vik Fearing
vik@postgresfriends.org
In reply to: Dagfinn Ilmari Mannsåker (#1)
Re: Adding argument names to aggregate functions

On 2/27/23 14:22, Dagfinn Ilmari Mannsåker wrote:

Hi hackers,

I'm sure I'm not the only one who can never remember which way around
the value and delimiter arguments go for string_agg() and has to look it
up in the manual every time. To make it more convenient, here's a patch
that adds proargnames to its pg_proc entries so that it can be seen with
a quick \df in psql.

I also added names to json(b)_object_agg() for good measure, even though
they're more obvious. The remaining built-in multi-argument aggregate
functions are the stats-related ones, where it's all just Y/X (but why
in that order?), so I didn't think it was necessary. If others feel more
strongly, I can add those too.

No comment on adding names for everything, but a big +1 for the ones
included here.
--
Vik Fearing

In reply to: Dagfinn Ilmari Mannsåker (#1)
Re: Adding argument names to aggregate functions

Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:

Hi hackers,

I'm sure I'm not the only one who can never remember which way around
the value and delimiter arguments go for string_agg() and has to look it
up in the manual every time. To make it more convenient, here's a patch
that adds proargnames to its pg_proc entries so that it can be seen with
a quick \df in psql.

Added to the 2023-07 commitfest:

https://commitfest.postgresql.org/43/4275/

- ilmari

#4Jim Jones
jim.jones@uni-muenster.de
In reply to: Dagfinn Ilmari Mannsåker (#3)
Re: Adding argument names to aggregate functions

On 12.04.23 19:53, Dagfinn Ilmari Mannsåker wrote:

Dagfinn Ilmari Mannsåker<ilmari@ilmari.org> writes:

Hi hackers,

I'm sure I'm not the only one who can never remember which way around
the value and delimiter arguments go for string_agg() and has to look it
up in the manual every time. To make it more convenient, here's a patch
that adds proargnames to its pg_proc entries so that it can be seen with
a quick \df in psql.

Added to the 2023-07 commitfest:

https://commitfest.postgresql.org/43/4275/

- ilmari

+1 for adding the argument names.

The patch needs a rebase though.. it no longer applies :

$ git apply
~/Downloads/0001-Add-argument-names-to-multi-argument-aggregates.patch
error: patch failed: src/include/catalog/pg_proc.dat:8899
error: src/include/catalog/pg_proc.dat: patch does not apply

Jim

In reply to: Jim Jones (#4)
1 attachment(s)
Re: Adding argument names to aggregate functions

Jim Jones <jim.jones@uni-muenster.de> writes:

On 12.04.23 19:53, Dagfinn Ilmari Mannsåker wrote:

Dagfinn Ilmari Mannsåker<ilmari@ilmari.org> writes:

Hi hackers,

I'm sure I'm not the only one who can never remember which way around
the value and delimiter arguments go for string_agg() and has to look it
up in the manual every time. To make it more convenient, here's a patch
that adds proargnames to its pg_proc entries so that it can be seen with
a quick \df in psql.

Added to the 2023-07 commitfest:

https://commitfest.postgresql.org/43/4275/

- ilmari

+1 for adding the argument names.

The patch needs a rebase though.. it no longer applies :

$ git apply
~/Downloads/0001-Add-argument-names-to-multi-argument-aggregates.patch
error: patch failed: src/include/catalog/pg_proc.dat:8899
error: src/include/catalog/pg_proc.dat: patch does not apply

Thanks for the heads-up, here's a rebased patch. I've also formatted
the lines to match what reformat_dat_file.pl wants. It also wanted to
reformat a bunch of other entries, but I left those alone.

- ilmari

Attachments:

v2-0001-Add-argument-names-to-multi-argument-aggregates.patchtext/x-diffDownload
From a6ff997fcea7aa7201318cb94db0173ea6efdf02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 27 Feb 2023 13:06:29 +0000
Subject: [RFC PATCH v2] Add argument names to multi-argument aggregates

This makes it easier to see which way around the arguments go when
using \dfa.  This is particularly relevant for string_agg(), but add
it to json(b)_object_agg() too for good measure.
---
 src/include/catalog/pg_proc.dat | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index b516cee8bd..b2db8d07e1 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5017,7 +5017,7 @@
 { oid => '3538', descr => 'concatenate aggregate input into a string',
   proname => 'string_agg', prokind => 'a', proisstrict => 'f',
   prorettype => 'text', proargtypes => 'text text',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{value,delimiter}', prosrc => 'aggregate_dummy' },
 { oid => '3543', descr => 'aggregate transition function',
   proname => 'bytea_string_agg_transfn', proisstrict => 'f',
   prorettype => 'internal', proargtypes => 'internal bytea bytea',
@@ -5029,7 +5029,7 @@
 { oid => '3545', descr => 'concatenate aggregate input into a bytea',
   proname => 'string_agg', prokind => 'a', proisstrict => 'f',
   prorettype => 'bytea', proargtypes => 'bytea bytea',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{value,delimiter}', prosrc => 'aggregate_dummy' },
 
 # To ASCII conversion
 { oid => '1845', descr => 'encode text from DB encoding to ASCII text',
@@ -8978,7 +8978,7 @@
 { oid => '3197', descr => 'aggregate input into a json object',
   proname => 'json_object_agg', prokind => 'a', proisstrict => 'f',
   provolatile => 's', prorettype => 'json', proargtypes => 'any any',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{key,value}', prosrc => 'aggregate_dummy' },
 { oid => '8955', descr => 'aggregate non-NULL input into a json object',
   proname => 'json_object_agg_strict', prokind => 'a', proisstrict => 'f',
   provolatile => 's', prorettype => 'json', proargtypes => 'any any',
@@ -9906,7 +9906,7 @@
   prosrc => 'jsonb_object_agg_finalfn' },
 { oid => '3270', descr => 'aggregate inputs into jsonb object',
   proname => 'jsonb_object_agg', prokind => 'a', proisstrict => 'f',
-  prorettype => 'jsonb', proargtypes => 'any any',
+  prorettype => 'jsonb', proargtypes => 'any any', proargnames => '{key,value}',
   prosrc => 'aggregate_dummy' },
 { oid => '8963', descr => 'aggregate non-NULL inputs into jsonb object',
   proname => 'jsonb_object_agg_strict', prokind => 'a', proisstrict => 'f',
-- 
2.39.2

#6Jim Jones
jim.jones@uni-muenster.de
In reply to: Dagfinn Ilmari Mannsåker (#5)
Re: Adding argument names to aggregate functions

On 14.04.23 12:03, Dagfinn Ilmari Mannsåker wrote:

Thanks for the heads-up, here's a rebased patch. I've also formatted
the lines to match what reformat_dat_file.pl wants. It also wanted to
reformat a bunch of other entries, but I left those alone.

- ilmari

The patch applies cleanly now and \df shows the argument names:

postgres=# \df string_agg
                                List of functions
   Schema   |    Name    | Result data type |     Argument data
types      | Type
------------+------------+------------------+------------------------------+------
 pg_catalog | string_agg | bytea            | value bytea, delimiter
bytea | agg
 pg_catalog | string_agg | text             | value text, delimiter
text   | agg
(2 rows)

postgres=# \df json_object_agg
                                List of functions
   Schema   |      Name       | Result data type |  Argument data
types   | Type
------------+-----------------+------------------+------------------------+------
 pg_catalog | json_object_agg | json             | key "any", value
"any" | agg
(1 row)

I'm wondering if there are some sort of guidelines that dictate when to
name an argument or not. It would be nice to have one for future reference.

I will mark the CF entry as "Read for Committer" and let the committers
decide if it's best to first create a guideline for that or not.

Best, Jim

#7Jim Jones
jim.jones@uni-muenster.de
In reply to: Jim Jones (#6)
Re: Adding argument names to aggregate functions

On 18.04.23 10:58, I wrote:

On 14.04.23 12:03, Dagfinn Ilmari Mannsåker wrote:

Thanks for the heads-up, here's a rebased patch. I've also formatted
the lines to match what reformat_dat_file.pl wants.  It also wanted to
reformat a bunch of other entries, but I left those alone.

- ilmari

The patch applies cleanly now and \df shows the argument names:

postgres=# \df string_agg
                                List of functions
   Schema   |    Name    | Result data type |     Argument data
types      | Type
------------+------------+------------------+------------------------------+------

 pg_catalog | string_agg | bytea            | value bytea, delimiter
bytea | agg
 pg_catalog | string_agg | text             | value text, delimiter
text   | agg
(2 rows)

postgres=# \df json_object_agg
                                List of functions
   Schema   |      Name       | Result data type |  Argument data
types   | Type
------------+-----------------+------------------+------------------------+------

 pg_catalog | json_object_agg | json             | key "any", value
"any" | agg
(1 row)

I'm wondering if there are some sort of guidelines that dictate when
to name an argument or not. It would be nice to have one for future
reference.

I will mark the CF entry as "Read for Committer" and let the
committers decide if it's best to first create a guideline for that or
not.

Best, Jim

I just saw that the patch is failing[1]https://api.cirrus-ci.com/v1/artifact/task/5881376021413888/meson_log/build/meson-logs/meson-log.txt on "macOS - Ventura - Meson".
Not sure if it is related to this patch though ..

[1]: https://api.cirrus-ci.com/v1/artifact/task/5881376021413888/meson_log/build/meson-logs/meson-log.txt
https://api.cirrus-ci.com/v1/artifact/task/5881376021413888/meson_log/build/meson-logs/meson-log.txt

In reply to: Jim Jones (#7)
Re: Adding argument names to aggregate functions

Jim Jones <jim.jones@uni-muenster.de> writes:

On 18.04.23 10:58, I wrote:

On 14.04.23 12:03, Dagfinn Ilmari Mannsåker wrote:

Thanks for the heads-up, here's a rebased patch. I've also formatted
the lines to match what reformat_dat_file.pl wants.  It also wanted to
reformat a bunch of other entries, but I left those alone.

- ilmari

The patch applies cleanly now and \df shows the argument names:

postgres=# \df string_agg
                                List of functions
   Schema   |    Name    | Result data type |     Argument data
types      | Type
------------+------------+------------------+------------------------------+------
 pg_catalog | string_agg | bytea            | value bytea, delimiter bytea | agg
 pg_catalog | string_agg | text             | value text, delimiter text   | agg
(2 rows)

postgres=# \df json_object_agg
                                List of functions
   Schema   |      Name       | Result data type |  Argument data
types   | Type
------------+-----------------+------------------+------------------------+------
 pg_catalog | json_object_agg | json             | key "any", value "any" | agg
(1 row)

I'm wondering if there are some sort of guidelines that dictate when
to name an argument or not. It would be nice to have one for future
reference.

I seemed to recall a patch to add arugment names to a bunch of functions
in the past, thinking that might have some guidance, but can't for the
life of me find it now.

I will mark the CF entry as "Read for Committer" and let the
committers decide if it's best to first create a guideline for that or
not.

Best, Jim

I just saw that the patch is failing[1] on "macOS - Ventura -
Meson". Not sure if it is related to this patch though ..

[1]
https://api.cirrus-ci.com/v1/artifact/task/5881376021413888/meson_log/build/meson-logs/meson-log.txt

Link to the actual job:

https://cirrus-ci.com/task/5881376021413888

The failure was:

[09:54:38.727] 216/262 postgresql:recovery / recovery/031_recovery_conflict ERROR 198.73s exit status 60

Looking at its log:

https://api.cirrus-ci.com/v1/artifact/task/5881376021413888/testrun/build/testrun/recovery/031_recovery_conflict/log/regress_log_031_recovery_conflict

we see:

timed out waiting for match: (?^:User was holding a relation lock for too long) at /Users/admin/pgsql/src/test/recovery/t/031_recovery_conflict.pl line 311.

That looks indeed completely unrelated to this patch.

- ilmari

#9Jim Jones
jim.jones@uni-muenster.de
In reply to: Dagfinn Ilmari Mannsåker (#8)
Re: Adding argument names to aggregate functions

On 18.04.23 12:27, Dagfinn Ilmari Mannsåker wrote:

Link to the actual job:
https://cirrus-ci.com/task/5881376021413888

The failure was:

[09:54:38.727] 216/262 postgresql:recovery / recovery/031_recovery_conflict ERROR 198.73s exit status 60

Looking at its log:

https://api.cirrus-ci.com/v1/artifact/task/5881376021413888/testrun/build/testrun/recovery/031_recovery_conflict/log/regress_log_031_recovery_conflict

we see:

timed out waiting for match: (?^:User was holding a relation lock for too long) at /Users/admin/pgsql/src/test/recovery/t/031_recovery_conflict.pl line 311.

That looks indeed completely unrelated to this patch.

Yes, that's what I suspected. The patch passes all tests now :)

I've marked the CF entry as "Ready for Committer".

Jim

#10Daniel Gustafsson
daniel@yesql.se
In reply to: Jim Jones (#9)
1 attachment(s)
Re: Adding argument names to aggregate functions

This patch no longer applied but had a fairly trivial conflict so I've attached
a rebased v3 addressing the conflict in the hopes of getting this further.

--
Daniel Gustafsson

Attachments:

v3-0001-Add-argument-names-to-multi-argument-aggregates.patchapplication/octet-stream; name=v3-0001-Add-argument-names-to-multi-argument-aggregates.patch; x-unix-mode=0644Download
From e3d90cdb8f99ce8371184aa2324b2c6ef349c07a Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Wed, 19 Jul 2023 09:53:57 +0200
Subject: [PATCH v3] Add argument names to multi-argument aggregates
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This makes it easier to see which way around the arguments go when
using \dfa.  This is particularly relevant for string_agg(), but add
it to json(b)_object_agg() too for good measure.

Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/877cw3jl8y.fsf@wibble.ilmari.org
---
 src/include/catalog/pg_proc.dat | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6996073989..8ae5bf3302 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4992,7 +4992,7 @@
 { oid => '3538', descr => 'concatenate aggregate input into a string',
   proname => 'string_agg', prokind => 'a', proisstrict => 'f',
   prorettype => 'text', proargtypes => 'text text',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{value,delimiter}', prosrc => 'aggregate_dummy' },
 { oid => '3543', descr => 'aggregate transition function',
   proname => 'bytea_string_agg_transfn', proisstrict => 'f',
   prorettype => 'internal', proargtypes => 'internal bytea bytea',
@@ -5004,7 +5004,7 @@
 { oid => '3545', descr => 'concatenate aggregate input into a bytea',
   proname => 'string_agg', prokind => 'a', proisstrict => 'f',
   prorettype => 'bytea', proargtypes => 'bytea bytea',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{value,delimiter}', prosrc => 'aggregate_dummy' },
 
 # To ASCII conversion
 { oid => '1845', descr => 'encode text from DB encoding to ASCII text',
@@ -8953,7 +8953,7 @@
 { oid => '3197', descr => 'aggregate input into a json object',
   proname => 'json_object_agg', prokind => 'a', proisstrict => 'f',
   provolatile => 's', prorettype => 'json', proargtypes => 'any any',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{key,value}', prosrc => 'aggregate_dummy' },
 { oid => '6280', descr => 'aggregate non-NULL input into a json object',
   proname => 'json_object_agg_strict', prokind => 'a', proisstrict => 'f',
   provolatile => 's', prorettype => 'json', proargtypes => 'any any',
@@ -9881,7 +9881,7 @@
   prosrc => 'jsonb_object_agg_finalfn' },
 { oid => '3270', descr => 'aggregate inputs into jsonb object',
   proname => 'jsonb_object_agg', prokind => 'a', proisstrict => 'f',
-  prorettype => 'jsonb', proargtypes => 'any any',
+  prorettype => 'jsonb', proargtypes => 'any any', proargnames => '{key,value}',
   prosrc => 'aggregate_dummy' },
 { oid => '6288', descr => 'aggregate non-NULL inputs into jsonb object',
   proname => 'jsonb_object_agg_strict', prokind => 'a', proisstrict => 'f',
-- 
2.32.1 (Apple Git-133)

In reply to: Daniel Gustafsson (#10)
1 attachment(s)
Re: Adding argument names to aggregate functions

Daniel Gustafsson <daniel@yesql.se> writes:

This patch no longer applied but had a fairly trivial conflict so I've attached
a rebased v3 addressing the conflict in the hopes of getting this further.

Thanks for the heads-up! Turns out the conflict was due to the new
json(b)_object_agg(_unique)(_strict) functions, which should also have
proargnames added. Here's an updated patch that does that.

- ilmari

Attachments:

v4-0001-Add-argument-names-to-multi-argument-aggregates.patchtext/x-diffDownload
From 2da3bada4f2a9425cbaa925a51f78773e4e16dfb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 27 Feb 2023 13:06:29 +0000
Subject: [PATCH v4] Add argument names to multi-argument aggregates

This makes it easier to see which way around the arguments go when
using \dfa.  This is particularly relevant for string_agg(), but add
it to json(b)_object_agg() too for good measure.
---
 src/include/catalog/pg_proc.dat | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6996073989..3e283671dc 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4992,7 +4992,7 @@
 { oid => '3538', descr => 'concatenate aggregate input into a string',
   proname => 'string_agg', prokind => 'a', proisstrict => 'f',
   prorettype => 'text', proargtypes => 'text text',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{value,delimiter}', prosrc => 'aggregate_dummy' },
 { oid => '3543', descr => 'aggregate transition function',
   proname => 'bytea_string_agg_transfn', proisstrict => 'f',
   prorettype => 'internal', proargtypes => 'internal bytea bytea',
@@ -5004,7 +5004,7 @@
 { oid => '3545', descr => 'concatenate aggregate input into a bytea',
   proname => 'string_agg', prokind => 'a', proisstrict => 'f',
   prorettype => 'bytea', proargtypes => 'bytea bytea',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{value,delimiter}', prosrc => 'aggregate_dummy' },
 
 # To ASCII conversion
 { oid => '1845', descr => 'encode text from DB encoding to ASCII text',
@@ -8953,21 +8953,22 @@
 { oid => '3197', descr => 'aggregate input into a json object',
   proname => 'json_object_agg', prokind => 'a', proisstrict => 'f',
   provolatile => 's', prorettype => 'json', proargtypes => 'any any',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{key,value}', prosrc => 'aggregate_dummy' },
 { oid => '6280', descr => 'aggregate non-NULL input into a json object',
   proname => 'json_object_agg_strict', prokind => 'a', proisstrict => 'f',
   provolatile => 's', prorettype => 'json', proargtypes => 'any any',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{key,value}', prosrc => 'aggregate_dummy' },
 { oid => '6281',
   descr => 'aggregate input into a json object with unique keys',
   proname => 'json_object_agg_unique', prokind => 'a', proisstrict => 'f',
   provolatile => 's', prorettype => 'json', proargtypes => 'any any',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{key,value}', prosrc => 'aggregate_dummy' },
 { oid => '6282',
   descr => 'aggregate non-NULL input into a json object with unique keys',
   proname => 'json_object_agg_unique_strict', prokind => 'a',
   proisstrict => 'f', provolatile => 's', prorettype => 'json',
-  proargtypes => 'any any', prosrc => 'aggregate_dummy' },
+  proargtypes => 'any any', proargnames => '{key,value}',
+  prosrc => 'aggregate_dummy' },
 { oid => '3198', descr => 'build a json array from any inputs',
   proname => 'json_build_array', provariadic => 'any', proisstrict => 'f',
   provolatile => 's', prorettype => 'json', proargtypes => 'any',
@@ -9881,22 +9882,22 @@
   prosrc => 'jsonb_object_agg_finalfn' },
 { oid => '3270', descr => 'aggregate inputs into jsonb object',
   proname => 'jsonb_object_agg', prokind => 'a', proisstrict => 'f',
-  prorettype => 'jsonb', proargtypes => 'any any',
+  prorettype => 'jsonb', proargtypes => 'any any', proargnames => '{key,value}',
   prosrc => 'aggregate_dummy' },
 { oid => '6288', descr => 'aggregate non-NULL inputs into jsonb object',
   proname => 'jsonb_object_agg_strict', prokind => 'a', proisstrict => 'f',
-  prorettype => 'jsonb', proargtypes => 'any any',
+  prorettype => 'jsonb', proargtypes => 'any any', proargnames => '{key,value}',
   prosrc => 'aggregate_dummy' },
 { oid => '6289',
   descr => 'aggregate inputs into jsonb object checking key uniqueness',
   proname => 'jsonb_object_agg_unique', prokind => 'a', proisstrict => 'f',
-  prorettype => 'jsonb', proargtypes => 'any any',
+  prorettype => 'jsonb', proargtypes => 'any any', proargnames => '{key,value}',
   prosrc => 'aggregate_dummy' },
 { oid => '6290',
   descr => 'aggregate non-NULL inputs into jsonb object checking key uniqueness',
   proname => 'jsonb_object_agg_unique_strict', prokind => 'a',
   proisstrict => 'f', prorettype => 'jsonb', proargtypes => 'any any',
-  prosrc => 'aggregate_dummy' },
+  proargnames => '{key,value}', prosrc => 'aggregate_dummy' },
 { oid => '3271', descr => 'build a jsonb array from any inputs',
   proname => 'jsonb_build_array', provariadic => 'any', proisstrict => 'f',
   provolatile => 's', prorettype => 'jsonb', proargtypes => 'any',
-- 
2.39.2

#12Daniel Gustafsson
daniel@yesql.se
In reply to: Dagfinn Ilmari Mannsåker (#11)
Re: Adding argument names to aggregate functions

On 19 Jul 2023, at 19:32, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

This patch no longer applied but had a fairly trivial conflict so I've attached
a rebased v3 addressing the conflict in the hopes of getting this further.

Thanks for the heads-up! Turns out the conflict was due to the new
json(b)_object_agg(_unique)(_strict) functions, which should also have
proargnames added. Here's an updated patch that does that.

Great, thanks! I had a quick look at this while rebasing (as well as your
updated patch) and it seems like a good idea to add this. Unless there are
objections I will look at getting this in.

--
Daniel Gustafsson

#13Nathan Bossart
nathandbossart@gmail.com
In reply to: Daniel Gustafsson (#12)
Re: Adding argument names to aggregate functions

On Wed, Jul 19, 2023 at 09:38:12PM +0200, Daniel Gustafsson wrote:

Great, thanks! I had a quick look at this while rebasing (as well as your
updated patch) and it seems like a good idea to add this. Unless there are
objections I will look at getting this in.

Hey Daniel, are you still planning on committing this? I can pick it up if
you are busy.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

#14Daniel Gustafsson
daniel@yesql.se
In reply to: Nathan Bossart (#13)
Re: Adding argument names to aggregate functions

On 4 Aug 2023, at 01:36, Nathan Bossart <nathandbossart@gmail.com> wrote:

On Wed, Jul 19, 2023 at 09:38:12PM +0200, Daniel Gustafsson wrote:

Great, thanks! I had a quick look at this while rebasing (as well as your
updated patch) and it seems like a good idea to add this. Unless there are
objections I will look at getting this in.

Hey Daniel, are you still planning on committing this? I can pick it up if
you are busy.

Finally unburied this from the post-vacation pile on the TODO list and pushed
it after another once-over.

--
Daniel Gustafsson

In reply to: Daniel Gustafsson (#14)
Re: Adding argument names to aggregate functions

On Thu, 24 Aug 2023, at 11:05, Daniel Gustafsson wrote:

On 4 Aug 2023, at 01:36, Nathan Bossart <nathandbossart@gmail.com> wrote:

On Wed, Jul 19, 2023 at 09:38:12PM +0200, Daniel Gustafsson wrote:

Great, thanks! I had a quick look at this while rebasing (as well as your
updated patch) and it seems like a good idea to add this. Unless there are
objections I will look at getting this in.

Hey Daniel, are you still planning on committing this? I can pick it up if
you are busy.

Finally unburied this from the post-vacation pile on the TODO list and pushed
it after another once-over.

Thanks!

--
- ilmari