Re: [HACKERS] extend pgbench expressions with functions

On Wed, Mar 30, 2016 at 1:23 AM, Fabien COELHO wrote: > > Hello Robert, > >>> If we don't nuke it, it'll never die. >> >> >> Hearing no objections, BOOM. > > > FIZZ! :-) > > Thanks for the commits, and apology for the portability bugs. Thanks for the additions, Fabien. This

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert, If we don't nuke it, it'll never die. Hearing no objections, BOOM. FIZZ! :-) Thanks for the commits, and apology for the portability bugs. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] extend pgbench expressions with functions

On Mon, Mar 28, 2016 at 9:36 PM, Stephen Frost wrote: > * Michael Paquier (michael.paqu...@gmail.com) wrote: >> On Tue, Mar 29, 2016 at 9:54 AM, Robert Haas wrote: >> > On Sun, Mar 27, 2016 at 5:01 AM, Fabien COELHO wrote: >> >>

Re: [HACKERS] extend pgbench expressions with functions

I guess the question here is whether we want the part-c patch, which removes the historical \setrandom syntax. That's technically no longer needed since we now can use random() as a function directly inside \set commands, but we might want it anyway for backward compatibility. This patch is

Re: [HACKERS] extend pgbench expressions with functions

* Michael Paquier (michael.paqu...@gmail.com) wrote: > On Tue, Mar 29, 2016 at 9:54 AM, Robert Haas wrote: > > On Sun, Mar 27, 2016 at 5:01 AM, Fabien COELHO wrote: > >> v40 is yet another rebase. > > > > Thanks. Committed after removing an

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Mar 29, 2016 at 9:54 AM, Robert Haas wrote: > On Sun, Mar 27, 2016 at 5:01 AM, Fabien COELHO wrote: >> v40 is yet another rebase. > > Thanks. Committed after removing an unnecessary parameter from the > coerceToXXX() functions. > > I guess the

Re: [HACKERS] extend pgbench expressions with functions

On Sun, Mar 27, 2016 at 5:01 AM, Fabien COELHO wrote: > v40 is yet another rebase. Thanks. Committed after removing an unnecessary parameter from the coerceToXXX() functions. I guess the question here is whether we want the part-c patch, which removes the historical

Re: [HACKERS] extend pgbench expressions with functions

v40 is yet another rebase. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index c6d1454..4ceddae 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -815,9 +815,10 @@ pgbench options dbname - Sets variable

Re: [HACKERS] extend pgbench expressions with functions

Bonjour Michaël, v39 is yet another rebase: 42 is in sight! What's up with v42? Is that your personal record? It is just a (geek) joke, see: https://en.wikipedia.org/wiki/42_%28number%29#Hitchhiker.27s_Guide_to_the_Galaxy It is the answer to the "Ultimate Question of Life, The Universe

Re: [HACKERS] extend pgbench expressions with functions

On Mon, Mar 21, 2016 at 6:56 AM, Fabien COELHO wrote: > >> v38 is a simple rebase, trying to keep up-to-date with Tom's work. > > > v39 is yet another rebase: 42 is in sight! What's up with v42? Is that your personal record? -- Michael -- Sent via pgsql-hackers mailing

Re: [HACKERS] extend pgbench expressions with functions

v38 is a simple rebase, trying to keep up-to-date with Tom's work. v39 is yet another rebase: 42 is in sight! -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index c6d1454..4ceddae 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++

Re: [HACKERS] extend pgbench expressions with functions

v38 is a simple rebase, trying to keep up-to-date with Tom's work. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index dd3fb1d..cf9c1cd 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -802,9 +802,10 @@ pgbench options

Re: [HACKERS] extend pgbench expressions with functions

Here is a v36 which inspect very carefully the string to decide whether it is an int or a double. You may, or may not, find it to your taste, I can't say. Here is a v37 which is mostly a rebase after recent changes. Also I noticed that I was double counting errors in the previous version, so

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert, [...] With your patch, you get different behavior depending on exactly how the input is malformed. I understand that you require only one possible error message on malformed input, instead of failing when converting to double if the input looked like a double (there was a '.'

Re: [HACKERS] extend pgbench expressions with functions

On Wed, Mar 9, 2016 at 3:09 AM, Fabien COELHO wrote: > I'm not sure what is "not acceptable" as it "totally breaks the error > handling" in the above code. > > I assumed that you want to check that sscanf can read what sprintf generated > when handling "\set". I'd guess that

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert, Here is a v35 b & c. This is not acceptable: + /* guess double type (n for "inf", "-inf" and "nan") */ + if (strchr(var, '.') != NULL || strchr(var, 'n') != NULL) + { +

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Mar 8, 2016 at 3:52 PM, Fabien COELHO wrote: > [ new patch and assorted color commentary ] This is not acceptable: + /* guess double type (n for "inf", "-inf" and "nan") */ + if (strchr(var, '.') != NULL ||

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert. Here is a v34 b & c. // comments are not allowed. I'd just remove the two you have. Back to the eighties! It make no sense to exit(1) and then return 0, so don't do that. I might write this code as: This would get rid of the internal-error case here altogether in favor of

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Mar 8, 2016 at 11:48 AM, Fabien COELHO wrote: > If this is a blocker I'll do them, but I'm convince it is not what should be > done. Well, I think it's a blocker. Exiting within deeply-nested code instead of propagating the error upward does not strike me as a good

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert, Having a look at 33-b, this looks pretty good now, but: // comments are not allowed. I'd just remove the two you have. Oops, C89 did not make it everywhere yet! It make no sense to exit(1) and then return 0, so don't do that. [...] This would get rid of the internal-error

Re: [HACKERS] extend pgbench expressions with functions

On Mon, Mar 7, 2016 at 4:58 AM, Fabien COELHO wrote: >> - 32-b: add double functions, including double variables >> - 32-c: remove \setrandom support (advice to use \set + random instead) > > Here is a rebased version after Tom's updates, 33-b & 33-c. I also extended > the

Re: [HACKERS] extend pgbench expressions with functions

- 32-b: add double functions, including double variables - 32-c: remove \setrandom support (advice to use \set + random instead) Here is a rebased version after Tom's updates, 33-b & 33-c. I also extended the floating point syntax to signed accept signed exponents, and changed the regexpr

Re: [HACKERS] extend pgbench expressions with functions

Attached is the fixed patch for the array method. Committed with a few tweaks, including running pgindent over some of it. Thanks. So the first set of functions is in, and the operators are executed as functions as well. Fabien, are you planning to send rebased versions of the rest? By that

Re: [HACKERS] extend pgbench expressions with functions

Committed with a few tweaks, including running pgindent over some of it. Thanks! -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] extend pgbench expressions with functions

On Wed, Mar 2, 2016 at 3:10 AM, Robert Haas wrote: > On Wed, Feb 17, 2016 at 3:22 AM, Fabien COELHO wrote: >> Indeed. My gcc 4.8.4 with --Wall does not show the warning, too bad. >> >> Attached is the fixed patch for the array method. > > Committed

Re: [HACKERS] extend pgbench expressions with functions

On Wed, Feb 17, 2016 at 3:22 AM, Fabien COELHO wrote: > Indeed. My gcc 4.8.4 with --Wall does not show the warning, too bad. > > Attached is the fixed patch for the array method. Committed with a few tweaks, including running pgindent over some of it. -- Robert Haas

Re: [HACKERS] extend pgbench expressions with functions

On Wed, Feb 17, 2016 at 5:22 PM, Fabien COELHO wrote: >> \set aid 1 + 1 >> pgbench -f addition.sql -t 5000 >> >> 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. > >

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, \set aid 1 + 1 pgbench -f addition.sql -t 5000 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. Yep, the execution trace is pretty similar in all cases, maybe with a little more

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Feb 16, 2016 at 11:12 PM, Fabien COELHO 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

Re: [HACKERS] extend pgbench expressions with functions

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

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Feb 16, 2016 at 5:18 AM, Fabien COELHO wrote: >>> Good point. One simple idea here would be to use a custom pgbench >>> script that has no SQL commands and just calculates the values of some >>> parameters to measure the impact without depending on the backend, >>>

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Feb 16, 2016 at 3:53 AM, Fabien COELHO wrote: > 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. > >

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert, Good point. One simple idea here would be to use a custom pgbench script that has no SQL commands and just calculates the values of some parameters to measure the impact without depending on the backend, with a fixed number of transactions. Sure, we could do that. But whether

Re: [HACKERS] extend pgbench expressions with functions

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,

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Feb 16, 2016 at 1:55 AM, Michael Paquier wrote: > On Tue, Feb 16, 2016 at 9:18 AM, Robert Haas wrote: >> I experimented with trying to do this and ran into a problem: where >> exactly would you store the evaluated arguments when you don't

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Feb 16, 2016 at 9:18 AM, Robert Haas wrote: > I experimented with trying to do this and ran into a problem: where > exactly would you store the evaluated arguments when you don't know > how many of them there will be? And even if you did know how many of > them

Re: [HACKERS] extend pgbench expressions with functions

On Mon, Feb 15, 2016 at 4:58 AM, Fabien COELHO wrote: > Indeed! > >> Taking patch 1 as a completely independent thing, there is no need to >> introduce PgBenchValueType yet. Similar remark for setIntValue and >> coerceToInt. They are useful afterwards when introducing double

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, + * Recursive evaluation of int or double expressions + * + * Note that currently only integer variables are available, with values + * stored as text. This comment is incorrect, we only care about integers in this patch. Indeed! Taking patch 1 as a completely independent

Re: [HACKERS] extend pgbench expressions with functions

On Mon, Feb 15, 2016 at 5:21 AM, Robert Haas wrote: > On Sun, Feb 14, 2016 at 11:28 AM, Fabien COELHO wrote: >> Here is a 3 part v29: >> >> a) add support for integer functions in pgbench, including turning >>operators as functions, as well as some

Re: [HACKERS] extend pgbench expressions with functions

On Sun, Feb 14, 2016 at 11:28 AM, Fabien COELHO wrote: > Here is a 3 part v29: > > a) add support for integer functions in pgbench, including turning >operators as functions, as well as some minimal infrastructure for >additional types (this allows to minimize the

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, I'll be happy if you do the review of the resulting split. OK, I am fine with this scenario as well. I have luckily done nothing yet. Here is a 3 part v29: a) add support for integer functions in pgbench, including turning operators as functions, as well as some minimal

Re: [HACKERS] extend pgbench expressions with functions

On Sun, Feb 14, 2016 at 4:42 PM, Fabien COELHO wrote: > >> So I would be fine to do a portion of the legwork and extract from this >> patch something smaller that adds only functions as a first step, with the >> minimum set of functions I mentioned upthread. Robert, Alvaro,

Re: [HACKERS] extend pgbench expressions with functions

On Sat, Feb 13, 2016 at 4:37 PM, Fabien COELHO wrote: > >> For example, I just realized that this patch allows values to be >> either a double or an integer and extends the operators to handle >> double values. But variables can still only be integers. > > Indeed. That's

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, - if the function & double stuff are separated ? - for the double part, if variables can be double ? I just double-checked and could not see a clear use case mentioned in this thread for double return types, Alas there is one: non uniform random functions which use a

Re: [HACKERS] extend pgbench expressions with functions

But variables can still only be integers. Version 28 attached has double variables, although typing is based on guessing because it is just a string. Any comment? [...] two separate enhancements in here. One of them is to support a new data type (doubles) and the other is to support

Re: [HACKERS] extend pgbench expressions with functions

On Sun, Feb 14, 2016 at 12:37 AM, Fabien COELHO wrote: > The two features are highly intermix, so it can only be dependent patches, > first to add a function infrastructure and probably some support for doubles > altough it would not be used, then to add doubles & their functions. > > A real pain

Re: [HACKERS] extend pgbench expressions with functions

On Sat, Feb 13, 2016 at 6:19 PM, Michael Paquier wrote: > On Sun, Feb 14, 2016 at 12:37 AM, Fabien COELHO wrote: >> The two features are highly intermix, so it can only be dependent patches, >> first to add a function infrastructure and probably some support for doubles

Re: [HACKERS] extend pgbench expressions with functions

On Sat, Feb 13, 2016 at 10:37 AM, Fabien COELHO wrote: > A real pain is the documentation, because it means writing a documentation > with only integer functions, then overwriting it with doubles. This is dumb > work, really, for the sake of "a cleaner git history", the

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert, You know, you make comments like this pretty much every time anybody suggests that you should change anything in any patch you write. Well, not everytime... I'm tired of doing and undoing things: I do a limited A because I'm cautious not to spend too much time that would go

Re: [HACKERS] extend pgbench expressions with functions

So I would be fine to do a portion of the legwork and extract from this patch something smaller that adds only functions as a first step, with the minimum set of functions I mentioned upthread. Robert, Alvaro, Fabien, does that sound fine to you? Thanks, but this is my c*, I have a few

Re: [HACKERS] extend pgbench expressions with functions

On Sun, Feb 14, 2016 at 10:05 AM, Robert Haas wrote: > On Sat, Feb 13, 2016 at 6:19 PM, Michael Paquier > wrote: >> On Sun, Feb 14, 2016 at 12:37 AM, Fabien COELHO wrote: >>> The two features are highly intermix, so it can only be dependent

Re: [HACKERS] extend pgbench expressions with functions

On Fri, Feb 12, 2016 at 5:06 AM, Fabien COELHO wrote: > I think that this option is too much bother for the small internal purpose > at hand. Yeah, I'm really frustrated with this whole thread. There's been a huge amount of discussion on this patch, but I don't really feel

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, Using a pointer to the tail of the list would make the code simple, and save a couple of lines. I did that, see v27 attached. Note that it does not save any lines, because the reverse function is removed, but then you need another type to keep the head & tail, the link type

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert, For example, I just realized that this patch allows values to be either a double or an integer and extends the operators to handle double values. But variables can still only be integers. Indeed. [...] at least flatten everything to string rather than integer so that you can

Re: [HACKERS] extend pgbench expressions with functions

On Thu, Feb 11, 2016 at 12:37 AM, Fabien COELHO wrote: > v26 attached implements these changes. +/* the argument list has been built in reverse order, it is fixed here */ +expr->u.function.args = reverse_elist(args); Hm. I may be missing something, but why is that

Re: [HACKERS] extend pgbench expressions with functions

On Fri, Feb 12, 2016 at 2:41 AM, Fabien COELHO wrote: > > Hello Michaël, > >> +/* the argument list has been built in reverse order, it is fixed >> here */ >> +expr->u.function.args = reverse_elist(args); >> Hm. I may be missing something, but why is that necessary?

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, +/* the argument list has been built in reverse order, it is fixed here */ +expr->u.function.args = reverse_elist(args); Hm. I may be missing something, but why is that necessary? This is basically doing a double-reversion to put all the arguments in the correct order

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Feb 9, 2016 at 5:06 AM, Alvaro Herrera wrote: > Fabien COELHO wrote: >> >> Hello, >> >> >>v23 attached, which does not change the message but does the other fixes. >> > >> >This doesn't apply anymore >> >> Indeed, but the latest version was really v25. >> >> >--

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, + parameter% of the time. Nitpick: double space here. Ok. + default: + return false; } In evalFunc(), the default case in switch for the operator functions should never be reached. Adding for example Assert(0) is

Re: [HACKERS] extend pgbench expressions with functions

Hello, v23 attached, which does not change the message but does the other fixes. This doesn't apply anymore Indeed, but the latest version was really v25. -- please rebase and submit to the next CF. I already provided it as v25 on Feb 1st. I closed it here as returned with feedback.

Re: [HACKERS] extend pgbench expressions with functions

Fabien COELHO wrote: > > Hello Michaël, > > v23 attached, which does not change the message but does the other fixes. This doesn't apply anymore -- please rebase and submit to the next CF. I closed it here as returned with feedback. Thanks! -- Álvaro Herrera

Re: [HACKERS] extend pgbench expressions with functions

Fabien COELHO wrote: > > Hello, > > >>v23 attached, which does not change the message but does the other fixes. > > > >This doesn't apply anymore > > Indeed, but the latest version was really v25. > > >-- please rebase and submit to the next CF. > > I already provided it as v25 on Feb 1st. >

Re: [HACKERS] extend pgbench expressions with functions

On Mon, Feb 1, 2016 at 9:46 PM, Michael Paquier wrote: > OK, here are patches for 9.1~9.4. The main differences are that in > 9.3/9.4 int64 is used for the division operations, and in 9.2/9.1 > that's int32. In the latter case pgbench blows up the same way with > that:

Re: [HACKERS] extend pgbench expressions with functions

On Wed, Feb 3, 2016 at 11:28 PM, Robert Haas wrote: > On Mon, Feb 1, 2016 at 9:46 PM, Michael Paquier > wrote: >> OK, here are patches for 9.1~9.4. The main differences are that in >> 9.3/9.4 int64 is used for the division operations, and in

Re: [HACKERS] extend pgbench expressions with functions

Yet another rebase, so as to propagate the same special case checks int for / and %. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index ade1b53..ebaf4e5 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -796,17 +796,21 @@

Re: [HACKERS] extend pgbench expressions with functions

On Mon, Feb 1, 2016 at 10:34 PM, Robert Haas wrote: > On Sat, Jan 30, 2016 at 7:36 AM, Michael Paquier > wrote: >> On Fri, Jan 29, 2016 at 11:21 PM, Fabien COELHO wrote: >>> +/* overflow check (needed for

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, - remove the short macros (although IMO it is a code degradation) FWIW, I like this suggestion from Robert. I'm not especially found of macros, my main reserve is that because of the length of the function names this necessarily creates lines longer than 80 columns or

Re: [HACKERS] extend pgbench expressions with functions

On Sat, Jan 30, 2016 at 7:36 AM, Michael Paquier wrote: > On Fri, Jan 29, 2016 at 11:21 PM, Fabien COELHO wrote: >> +/* overflow check (needed for INT64_MIN) */ >> +if (lval != 0 && (*retval < 0 == lval < 0)) >> >> Why not

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Feb 2, 2016 at 1:35 PM, Michael Paquier wrote: > And now there are patches. Well, nobody has complained about that until now > except me... So we could live without patching back-branches, but it don't > think it hurts much to fix those holes. Meh, s/it

Re: [HACKERS] extend pgbench expressions with functions

On Mon, Feb 1, 2016 at 9:46 PM, Michael Paquier wrote: > On Mon, Feb 1, 2016 at 10:34 PM, Robert Haas wrote: >> On Sat, Jan 30, 2016 at 7:36 AM, Michael Paquier >> wrote: >>> On Fri, Jan 29, 2016 at 11:21 PM, Fabien

Re: [HACKERS] extend pgbench expressions with functions

On Tue, Feb 2, 2016 at 1:24 PM, Robert Haas wrote: > On Mon, Feb 1, 2016 at 9:46 PM, Michael Paquier > wrote: > > On Mon, Feb 1, 2016 at 10:34 PM, Robert Haas > wrote: > >> On Sat, Jan 30, 2016 at 7:36 AM, Michael Paquier

Re: [HACKERS] extend pgbench expressions with functions

v22 compared to previous: - remove the short macros (although IMO it is a code degradation) - try not to remove/add blanks lines - let some assert "as is" - still exit on float to int overflow, see arguments in other mails - check for INT64_MIN / -1 (although I think it is useless) --

Re: [HACKERS] extend pgbench expressions with functions

On Fri, Jan 29, 2016 at 5:16 AM, Fabien COELHO < fabien.coe...@mines-paristech.fr> wrote: > v22 compared to previous: Thanks for the new patch! > - remove the short macros (although IMO it is a code degradation) FWIW, I like this suggestion from Robert. > - check for INT64_MIN / -1 (although

Re: [HACKERS] extend pgbench expressions with functions

On Fri, Jan 29, 2016 at 11:21 PM, Fabien COELHO wrote: > +/* overflow check (needed for INT64_MIN) */ > +if (lval != 0 && (*retval < 0 == lval < 0)) > > Why not use "if (lval == INT64_MIN)" instead of this complicated condition? > If it is really

Re: [HACKERS] extend pgbench expressions with functions

Also, a comment is needed to explain why such a bizarre condition is used/needed for just the INT64_MIN case. The last patch I sent has this bit: + /* + * Some machines throw a floating-point exception + * for INT64_MIN % -1, the correct answer being +

Re: [HACKERS] extend pgbench expressions with functions

On Fri, Jan 29, 2016 at 6:05 PM, Fabien COELHO wrote: > (instead of < in my previous suggestion, if some processors return 0 on > -INT64_MIN). Also, a comment is needed to explain why such a bizarre > condition is used/needed for just the INT64_MIN case. The last patch I

Re: [HACKERS] extend pgbench expressions with functions

Hi Michaël, I think it is overkill, but do as you feel. Perhaps we could have Robert decide on this one first? That's a bug after all that had better be backpatched. Fine with me. [modulo...] Right, forgot this one, we just need to check if rval is -1 here, and return 0 as result. I am

Re: [HACKERS] extend pgbench expressions with functions

So I'm arguing that exiting, with an error message, is better than handling user errors. I'm not objecting to exiting with an error message, but I think letting ourselves be killed by a signal is no good. Ok, I understand this point for this purpose. -- Fabien. -- Sent via pgsql-hackers

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, v23 attached, which does not change the message but does the other fixes. +if (coerceToInt() == INT64_MIN && coerceToInt() == -1) +{ + fprintf(stderr, "cannot divide INT64_MIN by -1\n"); + return false; +} Bike-shedding:

Re: [HACKERS] extend pgbench expressions with functions

On Fri, Jan 29, 2016 at 3:40 PM, Fabien COELHO wrote: > > I would as well suggest fixing first the (INT64_MAX / -1) crash on HEAD >> and back-branches with something like the patch attached, inspired from >> int8.c. >> > > I think it is overkill, but do as you feel. >

Re: [HACKERS] extend pgbench expressions with functions

v22 compared to previous: - remove the short macros (although IMO it is a code degradation) - try not to remove/add blanks lines - let some assert "as is" - still exit on float to int overflow, see arguments in other mails -

Re: [HACKERS] extend pgbench expressions with functions

I do not think that it is really worth fixing, but I will not prevent anyone to fix it. I still think it does. Well, if there is consensus to address this one and optionally the other integer overflows even on back branches, I'll write a patch and let's call that a deal. This is not a problem

Re: [HACKERS] extend pgbench expressions with functions

On Thu, Jan 28, 2016 at 1:36 AM, Fabien COELHO wrote: > So I'm arguing that exiting, with an error message, is better than handling > user errors. I'm not objecting to exiting with an error message, but I think letting ourselves be killed by a signal is no good. -- Robert

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert, And this one generates a core dump: \set cid debug(-9223372036854775808 / -1) Floating point exception: 8 (core dumped) That does not seem acceptable to me. I don't want pgbench to die a horrible death if a floating point exception occurs any more than I want the server to do

Re: [HACKERS] extend pgbench expressions with functions

On Thu, Jan 28, 2016 at 7:07 AM, Fabien COELHO wrote: > I do not think that it is really worth fixing, but I will not prevent anyone > to fix it. I still think it does. Well, if there is consensus to address this one and optionally the other integer overflows even on back

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert, Attached is a rebase after recent changes in pgbench code & doc. +/* use short names in the evaluator */ +#define INT(v) coerceToInt() +#define DOUBLE(v) coerceToDouble() +#define SET_INT(pv, ival) setIntValue(pv, ival) +#define SET_DOUBLE(pv, dval) setDoubleValue(pv, dval) I

Re: [HACKERS] extend pgbench expressions with functions

Hello Robert, Pgbench is a bench tool, not a production tool. I don't really see how that's relevant. My point is that I do not see any added value for pgbench to keep on executing a performance bench if some clients die due to script errors: it is more useful to stop the whole bench and

Re: [HACKERS] extend pgbench expressions with functions

On Wed, Jan 27, 2016 at 5:43 PM, Fabien COELHO wrote: > Pgbench is a bench tool, not a production tool. I don't really see how that's relevant. When I run a program and it dies after causing the operating system to send it a fatal signal, I say to myself "self, that program

Re: [HACKERS] extend pgbench expressions with functions

On Sat, Jan 16, 2016 at 1:10 PM, Fabien COELHO wrote: > All these results are fine from my point of view. > >> And this one generates a core dump: >> \set cid debug(-9223372036854775808 / -1) >> Floating point exception: 8 (core dumped) That does not seem acceptable to me.

Re: [HACKERS] extend pgbench expressions with functions

On Wed, Jan 27, 2016 at 3:39 AM, Fabien COELHO wrote: > Attached is a rebase after recent changes in pgbench code & doc. +/* use short names in the evaluator */ +#define INT(v) coerceToInt() +#define DOUBLE(v) coerceToDouble() +#define SET_INT(pv, ival) setIntValue(pv, ival)

Re: [HACKERS] extend pgbench expressions with functions

OK, so I had an extra look at this patch and I am marking it as ready for committer. Ok. Attached is a rebase after recent changes in pgbench code & doc. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 42d0667..d42208a 100644 ---

Re: [HACKERS] extend pgbench expressions with functions

The basic operator functions also do not check for integer overflows. This is a feature. I think that they should not check for overflow, as in C, this is just int64_t arithmetic "as is". (int64_t is not an available type on Windows btw.) Possibly. I really meant "64 bits signed

Re: [HACKERS] extend pgbench expressions with functions

OK, so I had an extra look at this patch and I am marking it as ready for committer. Ok. - INT64_MIN / -1 throws a core dump, and errors on HEAD. I think this should be fixed, Fabien does not. Yep. Another point about this one is that it is not related to this patch about functions. --

Re: [HACKERS] extend pgbench expressions with functions

On Mon, Jan 18, 2016 at 10:07 AM, Michael Paquier wrote: > On Sun, Jan 17, 2016 at 3:10 AM, Fabien COELHO wrote: >> I put the function evaluation in a function in the attached version. > > Thanks, this makes the code a bit clearer. OK, so I had an

Re: [HACKERS] extend pgbench expressions with functions

On Sun, Jan 17, 2016 at 3:10 AM, Fabien COELHO wrote: >> With this example: >> \set cid debug(sqrt(-1)) >> I get that: >> debug(script=0,command=1): double nan >> An error would be more logical, no? > > > If "sqrt(-1)" as a double is Nan for the computer, I'm fine with that.

Re: [HACKERS] extend pgbench expressions with functions

On Fri, Jan 15, 2016 at 11:53 PM, Fabien COELHO wrote: > Here is a v19 : > - avoid noisy changes > - abort on double->int overflow > - implement operators as functions > > There is still \setrandom, that I can remove easily with a green light. (I am not sure why *$%"#

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, + uniformly-distributed random integer in [lb,ub] Nitpick: when defining an interval like that, you may want to add a space after the comma. Why not. + /* beware that the list is reverse in make_func */ s/reverse/reversed/? Indeed. + #ifdef DEBUG Some

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, Here is a v19 : - avoid noisy changes - abort on double->int overflow - implement operators as functions There is still \setrandom, that I can remove easily with a green light. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index

Re: [HACKERS] extend pgbench expressions with functions

Hello Michaël, Hmmm, this is subjective:-) And dependent on personal opinions and views. I've decided to stay with the current behavior (\setrandom), that is to abort the current transaction on errors but not to abort pgbench itself. The check is done before calling the functions, and I

Re: [HACKERS] extend pgbench expressions with functions

On Thu, Jan 14, 2016 at 5:54 PM, Fabien COELHO wrote: > I wrote: >> - fprintf(stderr, "gaussian parameter must be at least >> %f (not \"%s\")\n", MIN_GAUSSIAN_PARAM, argv[5]); >> +fprintf(stderr, >> + "random gaussian

  1   2   >