Re: extend pgbench expressions with functions - Mailing list pgsql-hackers
From | Michael Paquier |
---|---|
Subject | Re: extend pgbench expressions with functions |
Date | |
Msg-id | CAB7nPqTJA-pQmM+QGAFazMAxPzhpMBq3TioixSa1h+ePUDBngQ@mail.gmail.com Whole thread Raw |
In response to | Re: extend pgbench expressions with functions (Fabien COELHO <fabien.coelho@mines-paristech.fr>) |
Responses |
Re: extend pgbench expressions with functions
|
List | pgsql-hackers |
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);
+typedef enum
+{
+ PGBT_NONE,
+ PGBT_INT,
+ PGBT_DOUBLE
+} PgBenchValueType;
+ if (retval->type == PGBT_INT)
+ fprintf(stderr, "int " INT64_FORMAT "\n", retval->u.ival);
+ else if (retval->type == PGBT_DOUBLE)
+ fprintf(stderr, "double %f\n", retval->u.dval);
+ else
+ fprintf(stderr, "none\n");
Another remaining item: should support for \setrandom be dropped? As the patch is presented this remains intact.
--
Michael
pgsql-hackers by date: