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

In response to

Responses

Browse pgsql-hackers by date

  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