On Tue, Feb 16, 2016 at 11:12 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
>
> Hello Robert,
>
>>> However, for obvious reasons the committer opinion prevails:-)
>>
>>
>> You're welcome to solicit other opinions. I'm not unwilling to give
>> way if there's a chorus of support for the way you've done it.
>
>
> Hmmm. I do not expect much chorus on such a minor subject:-)
>
>> But to me it sounds like you're saying that it doesn't really matter
>> whether the system is scalable and maintainable because we only have 5
>> functions right now, which is a design philosophy with which I just don't
>> agree.
>
>
> The design does not aim at scalability but at simplicity, otherwise I would
> have done things quite differently: with many functions the "switch" based
> selection does not scale anyway.
>
> Anyway, attached are two patches, one for each approach.
>
> The array (second patch) is not too bad if one agrees with a maximum number
> of arguments, and also as I did not change the list structure coming from
> the parser, so it does not need to manage the number of arguments in the
> function structure. The code for min/max is also simpler when dealing with
> an array instead of a linked list. I do not like much array references in
> the code, so I tried to avoid them by using lval/rval scalars for operator
> operands.
>
> Please choose the one you prefer, and I'll adapt the remaining stuff to your
> choice.
For those two patches and HEAD, it is possible to do a direct
performance comparison for simple operator functions, as those are
being switched to behave as functions. So doing the following for 50M
"transactions" on my laptop:
\set aid 1 + 1
pgbench -f addition.sql -t 50000000
I have the following:
HEAD: 3.5~3.7M TPS
list method: 3.6~3.7M TPS
array method: 3.4~3.5M TPS
So all approaches have a comparable performance.
Btw, patch 2 is returning a warning for me:
pgbench.c:1060:16: warning: comparison of constant
-9223372036854775808 with expression of type 'int' is always false
[-Wtautological-constant-out-of-range-compare] if (lval == PG_INT64_MIN)
It is trying to compare a 32b integer with an int64 value, evalFunc
needed an int64.
--
Michael