Re: extend pgbench expressions with functions

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: extend pgbench expressions with functions
Date: 2016-01-27 21:33:06
Message-ID: CA+TgmoaDz0x02wLvsGM0AUtJE-F6E55KpfOmXvmX-mmygzYw1Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jan 27, 2016 at 3:39 AM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
> 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.

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

/*
* 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.

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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-01-27 21:35:43 Re: Patch: ResourceOwner optimization for tables with many partitions
Previous Message Igal @ Lucee.org 2016-01-27 21:31:09 Re: Implementing a new Scripting Language