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 12:42:37 |
Message-ID: | CAFj8pRDDwkMcxMiPKNK3dsPCpQ2wv9vYG3_-JbyBiUbA9Lpntw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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?
> regards, tom lane
>
Attachment | Content-Type | Size |
---|---|---|
minmax_variadic-20190222.patch | text/x-patch | 8.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2019-02-22 12:45:38 | Re: speeding up planning with partitions |
Previous Message | Tomas Vondra | 2019-02-22 12:03:13 | Re: CPU costs of random_zipfian in pgbench |