pgbench - add option to show actual builtin script code

Started by Fabien COELHOalmost 7 years ago9 messages
#1Fabien COELHO
coelho@cri.ensmp.fr
1 attachment(s)

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: <builtin: select only>
\set aid random(1, 100000 * :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: <builtin: TPC-B (sort of)>
simple-update: <builtin: simple update>
select-only: <builtin: select only>

--
Fabien.

Attachments:

pgbench-builtin-list-1.patchtext/x-diff; name=pgbench-builtin-list-1.patchDownload
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 <optional> <replaceable>options</replaceable> </optional> <replaceable>d
       </listitem>
      </varlistentry>
 
-
      <varlistentry>
       <term><option>-c</option> <replaceable>clients</replaceable></term>
       <term><option>--client=</option><replaceable>clients</replaceable></term>
@@ -456,6 +455,16 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
        </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--list</option><replaceable>scriptname</replaceable></term>
+      <listitem>
+       <para>
+        Show the actual code of builtin script <replaceable>scriptname</replaceable>
+        on stderr, and exit immediately.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>-M</option> <replaceable>querymode</replaceable></term>
       <term><option>--protocol=</option><replaceable>querymode</replaceable></term>
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 }
#2Ibrar Ahmed
ibrar.ahmad@gmail.com
In reply to: Fabien COELHO (#1)
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

#3Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Ibrar Ahmed (#2)
1 attachment(s)
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.

Attachments:

pgbench-builtin-list-2.patchtext/x-diff; name=pgbench-builtin-list-2.patchDownload
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 <optional> <replaceable>options</replaceable> </optional> <replaceable>d
       </listitem>
      </varlistentry>
 
-
      <varlistentry>
       <term><option>-c</option> <replaceable>clients</replaceable></term>
       <term><option>--client=</option><replaceable>clients</replaceable></term>
@@ -617,6 +616,16 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--show-script</option><replaceable>scriptname</replaceable></term>
+      <listitem>
+       <para>
+        Show the actual code of builtin script <replaceable>scriptname</replaceable>
+        on stderr, and exit immediately.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>-t</option> <replaceable>transactions</replaceable></term>
       <term><option>--transactions=</option><replaceable>transactions</replaceable></term>
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 }
#4Ibrar Ahmed
ibrar.ahmad@gmail.com
In reply to: Fabien COELHO (#3)
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

#5Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Ibrar Ahmed (#4)
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.

#6Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Fabien COELHO (#5)
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 Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#7Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Andrew Dunstan (#6)
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.

#8Thomas Munro
thomas.munro@gmail.com
In reply to: Fabien COELHO (#7)
Re: pgbench - add option to show actual builtin script code

On Fri, Jul 12, 2019 at 4:20 AM Fabien COELHO <coelho@cri.ensmp.fr> 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

#9Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Thomas Munro (#8)
Re: pgbench - add option to show actual builtin script code

Committed, after pgindent. Thanks Fabien and Ibrar.

Thanks for the commit.

--
Fabien.