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

On 02/23/19 01:22, Pavel Stehule wrote:
> so 23. 2. 2019 v 4:50 odesílatel Chapman Flack <chap@anastigmatix.net>
> napsal:

>> In transformMinMaxExpr:
>> The assignment of funcname doesn't look right.

... it still doesn't.

>> Two new errors are elogs. ...
>> I am not sure if there is a way for user input to trigger the first one.
>> Perhaps it can stay an elog if not. In any case, s/to
>> determinate/determine/.
>>
> 
> It is +/- internal error and usually should not be touched - so there I
> prefer elog. I fix message

... still needs s/to //.

Can the sentence added to the doc be changed to "These functions support
passing parameters as an array after <literal>VARIADIC</literal> keyword."?
That is, s/supports/support/ and s/a/an/. I've done that after a couple of
recent patches, but it seems to be on springs.

> What about using macros?

Meh ... the macros look nicer, but still rely just as much on the compiler
to hoist the tests out of the loop. I suppose it probably can.

I wouldn't have thought it necessary to change the switch statements in
FigureColnameInternal or get_rule_expr ... cases with multiple labels are
seen often enough, and it's probably hard to do better.


Taking a step back ...


All of this still begs Tom's question about whether array_greatest/
array_least would be preferable.

I understand Pavel's point:

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

but, to advocate the other side for a moment, perhaps that could be viewed
as more of a documentation problem.

At bottom, the confusion potential that concerns Pavel exists because
these things look like variadic functions, and the manual calls them
"the GREATEST and LEAST functions", but they won't accept a VARIADIC array
parameter as a genuine variadic function would.

But that's hardly the only way these differ from normal functions.
You can't find them in pg_proc or call them through fmgr. In fact, they
share these non-function properties with all of their siblings in the
functions-conditional section of the manual. (Arguably, if GREATEST and
LEAST end up taking a VARIADIC array parameter, COALESCE will be wanting
one next. And indeed, there doesn't seem to be an existing
array_firstnonnull function for that job, either.) And these "functions"
have special, type-unifying argument rules (already documented in
typeconv-union-case), late argument evaluation in the case of COALESCE,
and so on.

In other words, any effort to make these animals act more like functions
will be necessarily incomplete, and differences will remain.

Maybe the confusion-potential could be fixed by having one more sentence
at the top of the whole functions-conditional section, saying "Some of
these resemble functions, but are better viewed as function-like
expressions, with special rules for their argument lists." Then the section
for GREATEST/LEAST could have a "see also" for array_greatest/array_least,
the COALESCE section a "see also" for array_firstnonnull, and those simple
functions could be written.

The special rules for these constructs don't really buy anything for the
array-argument flavors: you can't pass in an array with heterogeneous
types or late-evaluated values anyway, so ordinary functions would suffice.

From the outside, it would look tidy and parsimonious to just have
GREATEST(VARIADIC...)/LEAST(VARIADIC...)/COALESCE(VARIADIC...) and have
just one set of "function" names to remember, rather than separate
array_* variants. But the cost of that tidiness from the outside is an
implementation that has to frob half a dozen source files on the inside.

The approach with more parsimony indoors would be to just create a few
new ordinary functions, and add to the doc explaining why they are
different, and that would be a patch only needing to touch a couple files.

I have a feeling that the final decision on whether the indoor or outdoor
parsimony matters more will be made by Tom, or another committer; I find
myself seeing both sides, and not feeling insider-y enough myself to
pick one.

-Chap