Hello Robert,

Attached is a rebase after recent changes in pgbench code & doc.

+/* use short names in the evaluator */
+#define INT(v) coerceToInt(&v)
+#define DOUBLE(v) coerceToDouble(&v)
+#define SET_INT(pv, ival) setIntValue(pv, ival)
+#define SET_DOUBLE(pv, dval) setDoubleValue(pv, dval)

I don't like this at all.  It seems to me that this really obscures
the code.  The few extra characters are a small price to pay for not
having to go look up the macro definition to understand what the code
is doing.

Hmmm. Postgres indentation rules for "switch" are peculiar to say the least and make it hard to write code that stay under 80 columns. The coerceToInt function name looks pretty long (I would rather have toInt/toDbl/setInt/setDbl) but I was "told" to use that, so I'm trying to find a tradeoff with a macro. Obviously I can substitude and have rather long lines that I personally find much uglier.

The third hunk in pgbench.c unnecessary deletes a blank line.

Yep, that is possible.

       /*
        * inner expresion in (cut, 1] (if parameter > 0), rand in [0, 1)
+        * Assert((1.0 - cut) != 0.0);
        */
-       Assert((1.0 - cut) != 0.0);
       rand = -log(cut + (1.0 - cut) * uniform) / parameter;
+

Moving the Assert() into the comment seems like a bad plan.  If the
Assert is true, it shouldn't be commented out.  If it's not, it
shouldn't be there at all.

I put this assertion when I initially wrote this code, but I think that it is proven so I moved it in comment just as a reminder for someone who might touch anything that this must hold.

Commit e41beea0ddb74ef975f08b917a354ec33cb60830, which you wrote, went
to some trouble to display good context for error messages.  What you
have here seems like a huge step backwards:

+                       fprintf(stderr, "double to int overflow for
%f\n", dval);
+                       exit(1);

So, we're just going to give up on all of that error context reporting
that we added back then?  That would be sad.

Well, I'm a lazy programmer, so I'm trying to measure the benefit. IMO there is no benefit to better manage this case, especially as the various solution I thought of where either ugly and/or had a significant impact on the code.

Note that in the best case the error would be detected and reported and the client is stopped, and other clients go on... But then, if you started a bench and some clients die while running probably your results are meaningless, so my opinion is that you are better off with an exit than with some message that you may miss and performance results computed with much less clients than you asked for.

Pgbench is a bench tool, not a production tool.

--
Fabien.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to