Re: proposal: variadic argument support for least, greatest function - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: proposal: variadic argument support for least, greatest function
Date
Msg-id CAFj8pRAN8brvjRNCp+cZc-V_CzW0xhSeskk9H2Weiwo+jNVh4w@mail.gmail.com
Whole thread Raw
In response to Re: proposal: variadic argument support for least, greatest function  (Pavel Stehule <pavel.stehule@gmail.com>)
Responses Re: proposal: variadic argument support for least, greatest function
List pgsql-hackers


pá 22. 2. 2019 v 13:42 odesílatel Pavel Stehule <pavel.stehule@gmail.com> napsal:
Hi

čt 21. 2. 2019 v 22:05 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
Pavel Stehule <pavel.stehule@gmail.com> writes:
> čt 21. 2. 2019 v 3:20 odesílatel Chapman Flack <chap@anastigmatix.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

pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: Journal based VACUUM FULL
Next
From: Andres Freund
Date:
Subject: Re: unconstify equivalent for volatile