Re: extend pgbench expressions with functions - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: extend pgbench expressions with functions
Date
Msg-id alpine.DEB.2.10.1602160851030.29962@sto
Whole thread Raw
In response to Re: extend pgbench expressions with functions  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: extend pgbench expressions with functions
List pgsql-hackers
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.



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Remove or weaken hints about "effective resolution of sleep delays is 10 ms"?
Next
From: Kyotaro HORIGUCHI
Date:
Subject: Re: [PROPOSAL] VACUUM Progress Checker.