Re: extend pgbench expressions with functions

On Fri, Jan 29, 2016 at 5:16 AM, Fabien COELHO <fabien.coelho@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 I think it is useless)
>  - try not to remove/add blanks lines
>  - let some assert "as is"
>  - still exit on float to int overflow, see arguments in other mails

+make_op(const char *operator, PgBenchExpr *lexpr, PgBenchExpr *rexpr)
+{
+       return make_func(find_func(operator),
+                                        /* beware that the list is reversed in make_func */
+                                        make_elist(rexpr, make_elist(lexpr, NULL)));
+}
I think that this should use as argument the function ID, aka PGBENCH_ADD or similar instead of find_func() with an operator character. This saves a couple of instructions.

+ * - nargs: number of arguments (-1 is a special value for min & max)
My fault perhaps, it may be better to mention here that -1 means that min and max need at least one argument, and that the number of arguments is not fixed.

+                                       case PGBENCH_MOD:
+                                               if (coerceToInt(&rval) == 0)
                                                {
                                                        fprintf(stderr, "division by zero\n");
                                                        return false;
                                                }
-                                               *retval = lval % rval;
+                                               if (coerceToInt(&lval) == INT64_MIN && coerceToInt(&rval) == -1)
+                                               {
+                                                       fprintf(stderr, "cannot divide INT64_MIN by -1\n");
+                                                       return false;
+                                               }
For mod() there is no need to have an error, returning 0 is fine. You can actually do it unconditionally when rval == -1.

+               setDoubleValue(retval, d < 0.0? -d: d);

Another remaining item: should support for \setrandom be dropped? As the patch is presented this remains intact.
--
Michael