Re: extend pgbench expressions with functions
От | Fabien COELHO |
---|---|
Тема | Re: extend pgbench expressions with functions |
Дата | |
Msg-id | alpine.DEB.2.10.1603081733330.23770@sto обсуждение исходный текст |
Ответ на | Re: extend pgbench expressions with functions (Robert Haas <robertmhaas@gmail.com>) |
Ответы |
Re: extend pgbench expressions with functions
|
Список | pgsql-hackers |
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 case here altogether in favor > of testing it via an assertion. Ok. Fine with me. > I think that coerceToInt() should not exit(1) when an overflow occurs; > instead, I think the function should be declared as bool > coerceToInt(PgBenchValue *pval, int64 *result), and the error return > should be propagated back to the caller, which can then return false > as we do for other error cases. This is the pain (ugly repeated code) that I wish to avoid, because then we cannot write a simple addition for doing an addition, but have to do several ugly tests instead. Ok, elegance is probably not a sufficient argument against doing that. Moreover, I do not see any condition in which doing what you suggest makes much sense from the user perspective: if there is an internal error in the bench code it seems much more logical to ask for the user for a sensible bench script, because I would not know how to interpret tps on a bench with internal failures in the client code anyway. For me, exiting immediatly on internal script errors is the right action. If this is a blocker I'll do them, but I'm convince it is not what should be done. > I think that some of the places you've used coerceToInt() are not > appropriate. Like, for example: > > + if > (coerceToInt(rval) == 0) > + > fprintf(stderr, "division by zero\n"); > + return false; > + } > > Now, if rval is out of range of an integer, that is going to overflow > while trying to see whether it should divide by zero. Please work a > little harder here and in similar cases. Ok. > Maybe add a helper function > checkIntegerEquality(PgBenchValue *, int64). Maybe. -- Fabien.
В списке pgsql-hackers по дате отправления: