Hello Robert,
[...] But we can't have things that are logically part of patch #2 just
tossed in with patch #1.
So you want integer functions without type in patch #1.
I was in the middle of ripping that out of the patch when I realized
that evalFunc() is pretty badly designed.
Probably, we are just at v30:-)
What you've done is made it the job of each particular function to
evaluate its arguments.
Yep.
I did that for the multiple issues you raise below, and some you did not
yet foresee: handling a variable number of arguments (0, 1, 2, 3, n),
avoiding dynamic structures or arbitrary limitations, checking for a valid
number of arguments, and also the fact that the evaluation call was not
too horrible (2 lines per evaluation, factored out by groups of functions
[operators, min/max, randoms, ...], it is not fully repeated).
There are 5 sub-expression evaluation in the function, totalling 10 LOCs.
TEN.
I don't think that's a good plan.
The one you suggest does not strike me as intrinsically better: it is a
trade between some code ugliness (5 repeated evals = 10 lines, a little
more with the double functions, probably 20 lines) to other uglinesses
(number of args limit or dynamic allocation, array length to manage and
test somehow, list to array conversion code, things that will mean far
more than the few lines of repeated code under discussion).
So I think that it is just a choice between two plans, really, the better
of which is debatable.
I experimented with trying to do this and ran into a problem:
Yep. There are other problems, all of which solvable obviously, but which
means that a lot more that 10 lines will be added to avoid the 5*2 lines
repetition.
My opinion is that the submitted version is simple and fine for the
purpose, and the plan you suggest replaces 5*2 repetitions by a lot of
code and complexity, which is not desirable and should be avoided.
However, for obvious reasons the committer opinion prevails:-)
After considering the various points I raised above, could you confirm
that you do still require the implementation of this array approach before
I spend time doing such a thing?
I also went over your documentation changes.
Thanks, this looks better.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
