Re: proposal: variadic argument support for least, greatest function

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Chapman Flack <chap(at)anastigmatix(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: proposal: variadic argument support for least, greatest function
Date: 2019-02-22 19:57:53
Message-ID: CAFj8pRAN8brvjRNCp+cZc-V_CzW0xhSeskk9H2Weiwo+jNVh4w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

pá 22. 2. 2019 v 13:42 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
napsal:

> Hi
>
> čt 21. 2. 2019 v 22:05 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
>
>> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> > čt 21. 2. 2019 v 3:20 odesílatel Chapman Flack <chap(at)anastigmatix(dot)net>
>> > napsal:
>> >> I am not sure I have an answer to the objections being raised on
>> grounds
>> >> of taste. To me, it's persuasive that GREATEST and LEAST are described
>> in
>> >> the docco as functions, they are used much like variadic functions, and
>> >> this patch allows them to be used in the ways you would expect variadic
>> >> functions to be usable.
>>
>> > I wrote doc (just one sentence) and minimal test. Both can be enhanced.
>>
>> I remain of the opinion that this patch is a mess.
>>
>> I don't share Pavel's opinion that this is a clean user API, though
>> I'll grant that others might have different opinions on that.
>> I could hold my nose and overlook that if it led to a clean internal
>> implementation. But it doesn't: this patch just bolts a huge,
>> undocumented wart onto the side of MinMaxExpr. (The arguments are
>> in the args field, except when they aren't? And everyplace that
>> deals with MinMaxExpr needs to know that, as well as the fact that
>> the semantics are totally different? Ick.)
>>
>
> fixed
>
>
>> An example of the lack of care here is that the change in struct
>> ExprEvalStep breaks that struct's size constraint:
>>
>> * Inline data for the operation. Inline data is faster to access,
>> but
>> * also bloats the size of all instructions. The union should be
>> kept to
>> * no more than 40 bytes on 64-bit systems (so that the entire struct
>> is
>> * no more than 64 bytes, a single cacheline on common systems).
>>
>>
> fixed
>
>
>> Andres is going to be quite displeased if that gets committed ;-).
>>
>
> I hope so I solved all your objections. Now, the patch is really reduced.
>
>
>
>> I still say we should reject this and invent array_greatest/array_least
>> functions instead.
>>
>
> I am not against these functions, but these functions doesn't solve a
> confusing of some users, so LEAST, GREATEST looks like variadic functions,
> but it doesn't allow VARIADIC parameter.
>
> Comments, notes?
>

I am sending second variant (little bit longer, but the main code is not
repeated)

regards

Pavel

>
>
>> regards, tom lane
>>
>

Attachment Content-Type Size
minmax_variadic-20190222-2.patch text/x-patch 9.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-02-22 20:31:39 Re: unconstify equivalent for volatile
Previous Message Andrew Dunstan 2019-02-22 19:56:09 Re: Journal based VACUUM FULL