Re: extend pgbench expressions with functions

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: extend pgbench expressions with functions
Date: 2016-02-10 14:04:54
Message-ID: CAB7nPqS5jfdPG-nF_TV2w1FaDbKM6LJyFaio7pFF5Z08wKOCHg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Feb 9, 2016 at 5:06 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
> Fabien COELHO wrote:
>>
>> Hello,
>>
>> >>v23 attached, which does not change the message but does the other fixes.
>> >
>> >This doesn't apply anymore
>>
>> Indeed, but the latest version was really v25.
>>
>> >-- please rebase and submit to the next CF.
>>
>> I already provided it as v25 on Feb 1st.
>>
>> >I closed it here as returned with feedback.
>>
>> I turned it to "moved to next CF" as the patch is already on the queue.
>
> Ah, I didn't see v25 anywhere. What you did should be fine, thanks.

I just had another look at this patch.

+ <replaceable>parameter</>% of the time.
Nitpick: double space here.

+ switch (func)
{
[...]
+ }
+ default:
+ return false;
}
In evalFunc(), the default case in switch for the operator functions
should never be reached. Adding for example Assert(0) is something to
consider.

PGBT_NONE and PGBENCH_NONE are used nowhere. Why not removing them or
replace the code paths where they would be used by assertions to
trigger errors for future developments?

Other than that the patch looks in pretty good shape to me.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2016-02-10 14:42:00 Re: Updated backup APIs for non-exclusive backups
Previous Message Magnus Hagander 2016-02-10 14:04:12 Re: Updated backup APIs for non-exclusive backups