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.)
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).
Andres is going to be quite displeased if that gets committed ;-).
I still say we should reject this and invent array_greatest/array_least
functions instead.
regards, tom lane