Re: Adding argument names to aggregate functions

From: Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>
To: Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Adding argument names to aggregate functions
Date: 2023-04-18 10:27:54
Message-ID: 87sfcxv61h.fsf@wibble.ilmari.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Jim Jones <jim(dot)jones(at)uni-muenster(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Jones 2023-04-18 10:39:33 Re: Adding argument names to aggregate functions
Previous Message David Rowley 2023-04-18 09:50:21 Re: Incremental sort for access method with ordered scan support (amcanorderbyop)