č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.
