Re: proposal: variadic argument support for least, greatest function

so 23. 2. 2019 v 18:28 odesílatel Chapman Flack <chap(at)anastigmatix(dot)net>
napsal:

> On 02/23/19 01:22, Pavel Stehule wrote:
> > so 23. 2. 2019 v 4:50 odesílatel Chapman Flack <chap(at)anastigmatix(dot)net>
> > napsal:
>
> >> In transformMinMaxExpr:
> >> The assignment of funcname doesn't look right.
>
> ... it still doesn't.
>

fixed

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

fixed

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

reverted, I use a variables

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

This is goal of this small patch - do life little bit more easier and
little bit more consistent.

It is very small patch without risks or slowdowns. So I expect the cost of
this patch is small. Just it is small step forward to users.

I wrote "greatest", "least" support before I wrote variadic functions
support. If I wrote it in different order, probably, greatest, least
functions was pg_proc based. On second hand, the limitation of pg_proc
functions was motivation for variadic function support.

With my experience, I am not sure if better documentation does things
better. For some kind of users some smart magic is important. They don't
want to see implementation details.

In my car, i lost hope to understand completely to engine. I am not sure if
users should to know so we have three kind of functions - a) pg_proc based
functions, b) parser based functions (and looks like functions), c) parser
based functions (and looks like constant).

I know so is important to understand to things, but nobody can understand
to all. And then it is nice, so the things just works

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

sure, every time it is commiter's decision.

Thank you for precious review :)

please, see, attached patch

> -Chap
>