Thread: [PATCH] add long options to pgbench (submission 1)
This is mostly for reference to the next commitfest. This very minor patch adds a corresponding long option to all short (one letter) options of pgbench. In particular for connection options there is now --host --username --port options similar to the "psql" client. While I was at developing some small extensions to pgbench, ISTM that I could do that without much effort. Note that I'm not so sure about whether to chose singular or plural long option names. -- Fabien.
On Thu, May 2, 2013 at 1:59 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote: > This is mostly for reference to the next commitfest. > > This very minor patch adds a corresponding long option to all short (one > letter) options of pgbench. In particular for connection options there is > now --host --username --port options similar to the "psql" client. > > While I was at developing some small extensions to pgbench, ISTM that I > could do that without much effort. I don't really have an opinion on whether this is worth doing, but we'd probably want to update all of our client utilities, not just pgbench, if we did. > Note that I'm not so sure about whether to chose singular or plural long > option names. I've wondered that myself. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hello Robert, >> This very minor patch adds a corresponding long option to all short >> (one letter) options of pgbench. [...] > > I don't really have an opinion on whether this is worth doing, but we'd > probably want to update all of our client utilities, not just pgbench, > if we did. The current status is that "official" clients already have systematic long options. I have checked: psql, pg_dump, pg_dumpall, pg_restore, pg_basebackup, createdb, createuser, createlang. Possibly other commands in contrib do not have long option. As for the rational, when I type interactively I tend to use short options, but in a script I like long options so that I may not need to look them up in the documentation too often when reading the script later. The other rational is that adding long options is cheap and straightforward. -- Fabien.
New submission which put long options in alphabetical order, which seems more logical. This is for reference to the next commitfest. -- Fabien.
> New submission which put long options in alphabetical order, which seems > more logical. > > This is for reference to the next commitfest. When I applied your patch appeared the following messages: $ patch -p1 < /tmp/pgbench-longopts.patch patching file contrib/pgbench/pgbench.c Hunk #1 succeeded at 2109 (offset 29 lines). patching file doc/src/sgml/pgbench.sgml Hunk #2 succeeded at 172 (offset 12 lines). Hunk #3 FAILED at 170. Hunk #4 succeeded at 193 (offset 11 lines). Hunk #5 FAILED at 199. Hunk #13 succeeded at 344 (offset -33 lines). Hunk #14 succeeded at 367 (offset -33 lines). Hunk #15 succeeded at 382 (offset -33 lines). Hunk #16 succeeded at 395 (offset -33 lines). Hunk #17 succeeded at 407 (offset -33 lines). Hunk #18 succeeded at 422 (offset -33 lines). Hunk #19 succeeded at 432 (offset -33 lines). Hunk #20 succeeded at 442 (offset -33 lines). Hunk #21 succeeded at 454 (offset -33 lines). 2 out of 24 hunks FAILED -- saving rejects to file doc/src/sgml/pgbench.sgml.rej Please fix that and re-send the patch. Regards, ----- -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Blog sobre TI: http://fabriziomello.blogspot.com >> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello -- View this message in context: http://postgresql.1045698.n5.nabble.com/PATCH-add-long-options-to-pgbench-submission-1-tp5754115p5760039.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
> Please fix that and re-send the patch. Find attached diff wrt current master. -- Fabien.
On Thu, Jun 20, 2013 at 9:59 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Find attached diff wrt current master.Please fix that and re-send the patch.
Thanks.
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
On Thu, Jun 20, 2013 at 9:17 AM, Fabrízio de Royes Mello <fabriziomello@gmail.com> wrote: > On Thu, Jun 20, 2013 at 9:59 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote: >>> Please fix that and re-send the patch. >> Find attached diff wrt current master. > Thanks. I would like to solicit opinions on whether this is a good idea. I understand that the patch author thinks it's a good idea, and I don't have a strong position either way. But I want to hear what other people think. Anyone have an opinion? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2013-06-25 12:11:06 -0400, Robert Haas wrote: > On Thu, Jun 20, 2013 at 9:17 AM, Fabrízio de Royes Mello > <fabriziomello@gmail.com> wrote: > > On Thu, Jun 20, 2013 at 9:59 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote: > >>> Please fix that and re-send the patch. > >> Find attached diff wrt current master. > > Thanks. > > I would like to solicit opinions on whether this is a good idea. I > understand that the patch author thinks it's a good idea, and I don't > have a strong position either way. But I want to hear what other > people think. > > Anyone have an opinion? +1. When writing scripts I like to use the long options because that makes it easier to understand some time later. I don't really see a reason not to do this. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Robert Haas escribió: > On Thu, Jun 20, 2013 at 9:17 AM, Fabrízio de Royes Mello > <fabriziomello@gmail.com> wrote: > > On Thu, Jun 20, 2013 at 9:59 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote: > >>> Please fix that and re-send the patch. > >> Find attached diff wrt current master. > > Thanks. > > I would like to solicit opinions on whether this is a good idea. I > understand that the patch author thinks it's a good idea, and I don't > have a strong position either way. But I want to hear what other > people think. +1 from me on the general idea. Didn't check the actual patch. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Robert Haas <robertmhaas@gmail.com> writes:
> I would like to solicit opinions on whether this is a good idea. I
> understand that the patch author thinks it's a good idea, and I don't
> have a strong position either way. But I want to hear what other
> people think.
If it makes pgbench more consistent with psql's command line options,
it seems reasonable to me.
regards, tom lane
On Tue, Jun 25, 2013 at 12:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> I would like to solicit opinions on whether this is a good idea. I >> understand that the patch author thinks it's a good idea, and I don't >> have a strong position either way. But I want to hear what other >> people think. > > If it makes pgbench more consistent with psql's command line options, > it seems reasonable to me. OK, I think it does that. So that's three votes in favor and none opposed. I think I'd like to quibble with some of the names a bit, though. The patch adds --fill-factor, but I think we should spell it without the hyphen: --fillfactor. I think --quiet-log should be spelled --quiet. I think --connect for each connection is not very descriptive; maybe --connection-per-transaction or something, although that's kind of long. I think -M should be aliased to --protocol, not --query-mode. --skip-some-update is incorrectly pluralized; if that's what we're going to use, it should be --skip-some-updates. Alternatively, we could use --update-large-tables-only, which might make the intent more clear. On another note, it doesn't look like this updates the output of pgbench --help, which seems important. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
> I think I'd like to quibble with some of the names a bit, though. That is a good idea, because I'm not a native English speaker and I was not so sure for many options. > The patch adds --fill-factor, but I think we should spell it > without the hyphen: --fillfactor. Fine with me. > I think --quiet-log should be spelled --quiet. ISTM that --quiet usually means "not verbose on stdout", so I added log because this was specific to the log output, and that there may be a need for a --quiet option for stdout at some time. > I think --connect for each connection is not very descriptive; maybe > --connection-per-transaction or something, although that's kind of long. Yes, I think that it is too long. You have to type them! What about '--reconnect'? > I think -M should be aliased to --protocol, not --query-mode. Fine with me. > --skip-some-update is incorrectly pluralized; if that's > what we're going to use, it should be --skip-some-updates. Indeed. > Alternatively, we could use --update-large-tables-only, which might > make the intent more clear. Yep, but quite long. > On another note, it doesn't look like this updates the output of > pgbench --help, which seems important. Indeed, it should. Please find attached a v4 which takes into account most of your comments, but "very very long" option names. -- Fabien.
On Tue, Jun 25, 2013 at 3:09 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote: >> I think --quiet-log should be spelled --quiet. > > ISTM that --quiet usually means "not verbose on stdout", so I added log > because this was specific to the log output, and that there may be a need > for a --quiet option for stdout at some time. The output that is quieted by -q is not the log output produced by --log; it's the regular progress output on stdout/stderr. So I changed that, and committed this, with some further cosmetic changes. I made the formatting of the help message more like psql's help message, including adjusting pgbench to start the description of each option in the same column that psql does. This got rid of a lot of line breaks and IMHO makes the output of pgbench --help quite a bit more readable. I made stylistic adjustments to the documentation portion of the patch as well, again to match the markup used for psql. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Jun 27, 2013 at 10:02 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Jun 25, 2013 at 3:09 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote: >>> I think --quiet-log should be spelled --quiet. >> >> ISTM that --quiet usually means "not verbose on stdout", so I added log >> because this was specific to the log output, and that there may be a need >> for a --quiet option for stdout at some time. > > The output that is quieted by -q is not the log output produced by > --log; it's the regular progress output on stdout/stderr. > > So I changed that, and committed this, with some further cosmetic > changes. I made the formatting of the help message more like psql's > help message, including adjusting pgbench to start the description of > each option in the same column that psql does. This got rid of a lot > of line breaks and IMHO makes the output of pgbench --help quite a bit > more readable. I made stylistic adjustments to the documentation > portion of the patch as well, again to match the markup used for psql. In help messages: + " -s NUM, --scale NUM scaling factor\n" This should be "-s, --scale=NUM" for the sake of consistency with other options. Regards, -- Fujii Masao
On Thu, Jun 27, 2013 at 10:24 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Thu, Jun 27, 2013 at 10:02 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Tue, Jun 25, 2013 at 3:09 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote: >>>> I think --quiet-log should be spelled --quiet. >>> >>> ISTM that --quiet usually means "not verbose on stdout", so I added log >>> because this was specific to the log output, and that there may be a need >>> for a --quiet option for stdout at some time. >> >> The output that is quieted by -q is not the log output produced by >> --log; it's the regular progress output on stdout/stderr. >> >> So I changed that, and committed this, with some further cosmetic >> changes. I made the formatting of the help message more like psql's >> help message, including adjusting pgbench to start the description of >> each option in the same column that psql does. This got rid of a lot >> of line breaks and IMHO makes the output of pgbench --help quite a bit >> more readable. I made stylistic adjustments to the documentation >> portion of the patch as well, again to match the markup used for psql. > > In help messages: > > + " -s NUM, --scale NUM scaling factor\n" > > This should be "-s, --scale=NUM" for the sake of consistency with other > options. Woops, missed that one. Fixed, thanks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
> [...] So I changed that, and committed this, with some further cosmetic > changes. [...] Thanks for the commit & the style improvements. For the text formatting, I tried to keep the screen width under 80 characters, because if the line is too wide it is harder to read as the eye may loose the alignment. But being homogeneous with other commands is fine with me as well. -- Fabien.
Fabien COELHO escribió: > For the text formatting, I tried to keep the screen width under 80 > characters, because if the line is too wide it is harder to read as > the eye may loose the alignment. But being homogeneous with other > commands is fine with me as well. The format chosen by Robert fits in 80 columns and looks very good to me. Thanks to both. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services