On Tue, Feb 9, 2016 at 4:22 AM, Fabien COELHO <coe...@cri.ensmp.fr> wrote:
>> +       /* compute total_weight */
>> +       for (i = 0; i < num_scripts; i++)
>> +       {
>> +               total_weight += sql_script[i].weight;
>> +
>> +               /* detect overflow... */
>> If let as int64, you may want to remove this overflow check, or keep
>> it as int32.
> I'd rather keep int64, and remove the check.

OK, and you did so. Thanks.

>>> If you want something else, it would help to provide a sample of what you
>>> expect.
>> You could do that with an additional option here as well:
>> --output-format=normal|yamljson. The tastes of each user is different.
> I think that json/yaml-ifying pgbench output is beyond the object of this
> patch, so should be kept out?

Yeah, that's just a free idea that this set of patches does not need
to address. If someone thinks that's worth it, feel free to submit a
patch, perhaps we could add a TODO item on the wiki. Regarding the
output generated by your patch, I think that's fine. Perhaps Alvaro
has other thoughts on the matter. I don't know this part.

>>> Find attached a 18-d which addresses these concerns, and a actualized
>>> 18-e
>>> for the prefix.
>> (I could live without that personally)
> Hmmm, I type them and I'm not so good with a keyboard, so "se" is better
> than:
> "selct-only<back-back-back-back-back-back-back-back>ect-only".

I can understand that feeling.

>> -/* return builtin script "name", or fail if not found */
>> +/* return commands for selected builtin script, if unambiguous */
>> static script_t *
>> findBuiltin(const char *name)
>> This comment needs a refresh. This does not return a set of commands,
>> but the script itself.
> Indeed.
> Attached 19-d and 19-e.

+/* return builtin script "name", or fail if not found */
builtin does not sound like correct English to me, but built-in is.

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

Reply via email to