On Mon, Feb 9, 2015 at 2:54 PM, Tatsuo Ishii wrote:
> Agreed. Here is the patch to implement the idea: -f just implies -n.

Some small comments:
- is_no_vacuum, as well as is_init_mode, are defined as an integers
but their use imply that they are boolean switches. This patch sets
is_no_vacuum to true, while the rest of the code actually increment
its value when -n is used. Why not simply changing both flags as
booleans? My suggestion is not really related to this patch and purely
cosmetic but we could change things at the same time, or update your
patch to increment to is_no_vacuum++ when -f is used.
- The documentation misses some markups for pgbench and VACUUM and did
not respect the 80-character limit.
Attached is an updated patch with those things changed.
Regards,
-- 
Michael
From 3553151908a1be40b67703fcc27b696bad546b96 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Tue, 10 Feb 2015 03:59:19 +0900
Subject: [PATCH] Imply -n when using -f in pgbench

At the same time make the flags is_init_mode and is_no_vaccuum booleans.
This is a purely cosmetic change but those flags have been always used
as such, even if they were defined as integers.
---
 contrib/pgbench/pgbench.c | 9 +++++----
 doc/src/sgml/pgbench.sgml | 7 +++++++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index ddd11a0..eed9324 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -2679,8 +2679,8 @@ main(int argc, char **argv)
 	int			c;
 	int			nclients = 1;	/* default number of simulated clients */
 	int			nthreads = 1;	/* default number of threads */
-	int			is_init_mode = 0;		/* initialize mode? */
-	int			is_no_vacuum = 0;		/* no vacuum at all before testing? */
+	bool		is_init_mode = false;	/* initialize mode? */
+	bool		is_no_vacuum = false;	/* no vacuum at all before testing? */
 	int			do_vacuum_accounts = 0; /* do vacuum accounts before testing? */
 	int			ttype = 0;		/* transaction type. 0: TPC-B, 1: SELECT only,
 								 * 2: skip update of branches and tellers */
@@ -2753,13 +2753,13 @@ main(int argc, char **argv)
 		switch (c)
 		{
 			case 'i':
-				is_init_mode++;
+				is_init_mode = true;
 				break;
 			case 'h':
 				pghost = pg_strdup(optarg);
 				break;
 			case 'n':
-				is_no_vacuum++;
+				is_no_vacuum = true;
 				break;
 			case 'v':
 				do_vacuum_accounts++;
@@ -2872,6 +2872,7 @@ main(int argc, char **argv)
 			case 'f':
 				benchmarking_option_set = true;
 				ttype = 3;
+				is_no_vacuum = true;	/* "-f" implies "-n" (no vacuum) */
 				filename = pg_strdup(optarg);
 				if (process_file(filename) == false || *sql_files[num_files - 1] == NULL)
 					exit(1);
diff --git a/doc/src/sgml/pgbench.sgml b/doc/src/sgml/pgbench.sgml
index 7d203cd..0070882 100644
--- a/doc/src/sgml/pgbench.sgml
+++ b/doc/src/sgml/pgbench.sgml
@@ -316,6 +316,13 @@ pgbench <optional> <replaceable>options</> </optional> <replaceable>dbname</>
         <option>-N</option>, <option>-S</option>, and <option>-f</option>
         are mutually exclusive.
        </para>
+       <para>
+        Please note that <option>-f</option> implies <option>-n</option>,
+        which means that <command>VACUUM</command> for the standard
+        <application>pgbench</application> tables will not be performed before
+        running the benchmarking. <command>VACUUM</command> command should
+        be run beforehand if needed.
+       </para>
       </listitem>
      </varlistentry>
 
-- 
2.3.0

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

Reply via email to