From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: pgbench stats per script & other stuff
Date: Wed, 13 Jan 2016 14:37:27 +0100 (CET)
Message-ID: <alpine.DEB.2.10.1601131343531.2188@sto> (raw)
In-Reply-To: <20160112223220.GA882681@alvherre.pgsql>
References: <20160112223220.GA882681@alvherre.pgsql>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
Hello Alvaro,
>> Here is a two part v12, which:
>>
>> part a (refactoring of scripts and their stats):
>> - fix option checks (-i alone)
>> - s/repleacable/replaceable/ in doc
>> - keep small description in doc and help for -S & -N
>> - fix 2 comments for pg style
>> - show builtin list if not found
>
> I'm looking at this part of your patch and I think it's far too big to
> be a simple refactoring. Would you split it up please?
> I think the StatsData / SimpleStat addition should be one patch;
> then there's the -b changes. Then there may (or may not) be a bunch of
> other minor cleanups, not sure.
>
> I'm willing to commit these patches if I can easily review what they do,
> which I cannot with the current state.
Hmmm. ISTM that other people already reviewed it.
I can try to separate (again) some stuff, but there will be no miracle.
The overdue refactoring is because pgbench collects statistics at various
levels, and each time this is done in a different way. Cleaning this
requires to touch the stuff in many places, which means a "big" patch,
although ISTM a straightforward one, but this already the case with this
one.
> Please pgindent; make sure to add /*--- here to avoid pgindent mangling
> the comment:
Ok.
>> part b (weight)
>> - check that the weight is an int
>
> This part looks okay to me. Minor nitpick,
>
> + int i = 0, w = 0, wc = (int) getrand(thread, 0, total_weight - 1);
>
> should be three lines, not one.
Ok.
> Also the @W part in the --help output should be in brackets, as
> FILE[@W], right?
Why not.
--
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 instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: coelho@cri.ensmp.fr, alvherre@2ndquadrant.com
Subject: Re: pgbench stats per script & other stuff
In-Reply-To: <alpine.DEB.2.10.1601131343531.2188@sto>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox