Re: pgbench - add option to show actual builtin script code
Committed, after pgindent. Thanks Fabien and Ibrar. Thanks for the commit. -- Fabien.
Re: pgbench - add option to show actual builtin script code
On Fri, Jul 12, 2019 at 4:20 AM Fabien COELHO wrote: > >>> Now the patch is good now. > >>> > >>> The new status of this patch is: Ready for Committer > > > > Why aren't we instead putting the exact scripts in the documentation? > > Having to call pgbench with a special flag to get the script text seems > > a bit odd. > > A typical use case I had is to create a new script by modifying an > existing one for testing or debug. I prefer "command > file.sql ; vi > file.sql" to hazardous copy-pasting stuff from html pages. > > I do not think that it is worth replicating all scripts inside the doc, > they are not that interesting, especially if more are added. Currently, > out of the 3 scripts, only one is in the doc, and nobody complained:-) > > Now, they could be added to the documentation, but I'd like the option > anyway. Committed, after pgindent. Thanks Fabien and Ibrar. -- Thomas Munro https://enterprisedb.com
Re: pgbench - add option to show actual builtin script code
Hello Andrew, Now the patch is good now. The new status of this patch is: Ready for Committer Why aren't we instead putting the exact scripts in the documentation? Having to call pgbench with a special flag to get the script text seems a bit odd. A typical use case I had is to create a new script by modifying an existing one for testing or debug. I prefer "command > file.sql ; vi file.sql" to hazardous copy-pasting stuff from html pages. I do not think that it is worth replicating all scripts inside the doc, they are not that interesting, especially if more are added. Currently, out of the 3 scripts, only one is in the doc, and nobody complained:-) Now, they could be added to the documentation, but I'd like the option anyway. -- Fabien.
Re: pgbench - add option to show actual builtin script code
On 5/2/19 12:35 PM, Fabien COELHO wrote: > >> Now the patch is good now. >> >> The new status of this patch is: Ready for Committer > > Ok, thanks. > Why aren't we instead putting the exact scripts in the documentation? Having to call pgbench with a special flag to get the script text seems a bit odd. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pgbench - add option to show actual builtin script code
Now the patch is good now. The new status of this patch is: Ready for Committer Ok, thanks. -- Fabien.
Re: pgbench - add option to show actual builtin script code
Now the patch is good now. The new status of this patch is: Ready for Committer
Re: pgbench - add option to show actual builtin script code
Hello,
Patch looks good to me, and work fine on my machine. One minor
observation is option 'list' mostly used to list the elements like
"pgbench -b list" shows the available builtin scripts. Therefore we
should use a word which seems to be more relevant like --show-script.
Thanks for the review.
Here is a version with "--show-script". I also thought about "--listing",
maybe.
The new status of this patch is: Waiting on Author
--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index ee2501be55..c48593cfb9 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -355,7 +355,6 @@ pgbench options d
-
-c clients
--client=clients
@@ -617,6 +616,16 @@ pgbench options d
+
+ --show-scriptscriptname
+
+
+Show the actual code of builtin script scriptname
+on stderr, and exit immediately.
+
+
+
+
-t transactions
--transactions=transactions
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index a03ab281a5..55e6f6464a 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -651,6 +651,7 @@ usage(void)
" --progress-timestamp use Unix epoch timestamps for progress\n"
" --random-seed=SEED set random seed (\"time\", \"rand\", integer)\n"
" --sampling-rate=NUM fraction of transactions to log (e.g., 0.01 for 1%%)\n"
+ " --show-script=NAME show builtin script code, then exit\n"
"\nCommon options:\n"
" -d, --debug print debugging output\n"
" -h, --host=HOSTNAME database server host or socket directory\n"
@@ -4648,7 +4649,7 @@ listAvailableScripts(void)
fprintf(stderr, "Available builtin scripts:\n");
for (i = 0; i < lengthof(builtin_script); i++)
- fprintf(stderr, "\t%s\n", builtin_script[i].name);
+ fprintf(stderr, " %13s: %s\n", builtin_script[i].name, builtin_script[i].desc);
fprintf(stderr, "\n");
}
@@ -5088,6 +5089,7 @@ main(int argc, char **argv)
{"log-prefix", required_argument, NULL, 7},
{"foreign-keys", no_argument, NULL, 8},
{"random-seed", required_argument, NULL, 9},
+ {"show-script", required_argument, NULL, 10},
{NULL, 0, NULL, 0}
};
@@ -5440,6 +5442,13 @@ main(int argc, char **argv)
exit(1);
}
break;
+ case 10: /* list */
+{
+ const BuiltinScript *s = findBuiltin(optarg);
+ fprintf(stderr, "-- %s: %s\n%s\n", s->name, s->desc, s->script);
+ exit(0);
+}
+break;
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl b/src/bin/pgbench/t/002_pgbench_no_server.pl
index 69a6d03bb3..f7fa18418b 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -218,6 +218,15 @@ pgbench(
],
'pgbench builtin list');
+# builtin listing
+pgbench(
+ '--show-script se',
+ 0,
+ [qr{^$}],
+ [ qr{select-only: }, qr{SELECT abalance FROM pgbench_accounts WHERE},
+ qr{(?!UPDATE)}, qr{(?!INSERT)} ],
+ 'pgbench builtin listing');
+
my @script_tests = (
# name, err, { file => contents }
Re: pgbench - add option to show actual builtin script code
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed Patch looks good to me, and work fine on my machine. One minor observation is option 'list' mostly used to list the elements like "pgbench -b list" shows the available builtin scripts. Therefore we should use a word which seems to be more relevant like --show-script. The new status of this patch is: Waiting on Author
pgbench - add option to show actual builtin script code
Hello devs,
The minor attached patch $SUBJECT, so that it can be inspected easily,
instead of having to look at the source code or whatever.
sh> pgbench --list select-only
-- select-only:
\set aid random(1, 10 * :scale)
SELECT abalance FROM pgbench_accounts WHERE aid = :aid;
The builtin list output is also slightly improved:
sh> pgbench -b list
Available builtin scripts:
tpcb-like:
simple-update:
select-only:
--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index ee2501be55..4b7368b05e 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -355,7 +355,6 @@ pgbench options d
-
-c clients
--client=clients
@@ -456,6 +455,16 @@ pgbench options d
+
+ --listscriptname
+
+
+Show the actual code of builtin script scriptname
+on stderr, and exit immediately.
+
+
+
+
-M querymode
--protocol=querymode
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 99529de52a..9f085508d9 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -651,6 +651,7 @@ usage(void)
" --progress-timestamp use Unix epoch timestamps for progress\n"
" --random-seed=SEED set random seed (\"time\", \"rand\", integer)\n"
" --sampling-rate=NUM fraction of transactions to log (e.g., 0.01 for 1%%)\n"
+ " --list=NAME show builtin script code, then exit\n"
"\nCommon options:\n"
" -d, --debug print debugging output\n"
" -h, --host=HOSTNAME database server host or socket directory\n"
@@ -4658,7 +4659,7 @@ listAvailableScripts(void)
fprintf(stderr, "Available builtin scripts:\n");
for (i = 0; i < lengthof(builtin_script); i++)
- fprintf(stderr, "\t%s\n", builtin_script[i].name);
+ fprintf(stderr, " %13s: %s\n", builtin_script[i].name, builtin_script[i].desc);
fprintf(stderr, "\n");
}
@@ -5095,6 +5096,7 @@ main(int argc, char **argv)
{"log-prefix", required_argument, NULL, 7},
{"foreign-keys", no_argument, NULL, 8},
{"random-seed", required_argument, NULL, 9},
+ {"list", required_argument, NULL, 10},
{NULL, 0, NULL, 0}
};
@@ -5447,6 +5449,13 @@ main(int argc, char **argv)
exit(1);
}
break;
+ case 10: /* list */
+{
+ const BuiltinScript *s = findBuiltin(optarg);
+ fprintf(stderr, "-- %s: %s\n%s\n", s->name, s->desc, s->script);
+ exit(0);
+}
+break;
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl b/src/bin/pgbench/t/002_pgbench_no_server.pl
index 69a6d03bb3..d9cf01233c 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -218,6 +218,15 @@ pgbench(
],
'pgbench builtin list');
+# builtin listing
+pgbench(
+ '--list se',
+ 0,
+ [qr{^$}],
+ [ qr{select-only: }, qr{SELECT abalance FROM pgbench_accounts WHERE},
+ qr{(?!UPDATE)}, qr{(?!INSERT)} ],
+ 'pgbench builtin listing');
+
my @script_tests = (
# name, err, { file => contents }
