Fabien,

> I have not tested, but the patch looks ok in principle.

Thanks for the review. I have registered it to Aug Commit fest.
https://commitfest.postgresql.org/action/patch_view?id=1532

> I'm not sure of the variable name "is_non_init_parameter_set". I would 
> suggest "benchmarking_option_set"?

Ok, I will replace the variable name as you suggested.

> Also, to be consistent, maybe it should check that no initialization-specific 
> option are set when benchmarking.

Good suggesition. Here is the v2 patch.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index c0e5e24..67d7960 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -2520,6 +2520,9 @@ main(int argc, char **argv)
 	char	   *filename = NULL;
 	bool		scale_given = false;
 
+	bool		benchmarking_option_set = false;
+	bool		initializing_option_set = false;
+
 	CState	   *state;			/* status of clients */
 	TState	   *threads;		/* array of thread */
 
@@ -2599,12 +2602,14 @@ main(int argc, char **argv)
 				break;
 			case 'S':
 				ttype = 1;
+				benchmarking_option_set = true;
 				break;
 			case 'N':
 				ttype = 2;
+				benchmarking_option_set = true;
 				break;
 			case 'c':
-				nclients = atoi(optarg);
+				benchmarking_option_set = true;
 				if (nclients <= 0 || nclients > MAXCLIENTS)
 				{
 					fprintf(stderr, "invalid number of clients: %d\n", nclients);
@@ -2629,6 +2634,7 @@ main(int argc, char **argv)
 #endif   /* HAVE_GETRLIMIT */
 				break;
 			case 'j':			/* jobs */
+				benchmarking_option_set = true;
 				nthreads = atoi(optarg);
 				if (nthreads <= 0)
 				{
@@ -2637,9 +2643,11 @@ main(int argc, char **argv)
 				}
 				break;
 			case 'C':
+				benchmarking_option_set = true;
 				is_connect = true;
 				break;
 			case 'r':
+				benchmarking_option_set = true;
 				is_latencies = true;
 				break;
 			case 's':
@@ -2652,6 +2660,7 @@ main(int argc, char **argv)
 				}
 				break;
 			case 't':
+				benchmarking_option_set = true;
 				if (duration > 0)
 				{
 					fprintf(stderr, "specify either a number of transactions (-t) or a duration (-T), not both.\n");
@@ -2665,6 +2674,7 @@ main(int argc, char **argv)
 				}
 				break;
 			case 'T':
+				benchmarking_option_set = true;
 				if (nxacts > 0)
 				{
 					fprintf(stderr, "specify either a number of transactions (-t) or a duration (-T), not both.\n");
@@ -2681,12 +2691,14 @@ main(int argc, char **argv)
 				login = pg_strdup(optarg);
 				break;
 			case 'l':
+				benchmarking_option_set = true;
 				use_log = true;
 				break;
 			case 'q':
 				use_quiet = true;
 				break;
 			case 'f':
+				benchmarking_option_set = true;
 				ttype = 3;
 				filename = pg_strdup(optarg);
 				if (process_file(filename) == false || *sql_files[num_files - 1] == NULL)
@@ -2696,6 +2708,8 @@ main(int argc, char **argv)
 				{
 					char	   *p;
 
+					benchmarking_option_set = true;
+
 					if ((p = strchr(optarg, '=')) == NULL || p == optarg || *(p + 1) == '\0')
 					{
 						fprintf(stderr, "invalid variable definition: %s\n", optarg);
@@ -2708,6 +2722,7 @@ main(int argc, char **argv)
 				}
 				break;
 			case 'F':
+				initializing_option_set = true;
 				fillfactor = atoi(optarg);
 				if ((fillfactor < 10) || (fillfactor > 100))
 				{
@@ -2716,6 +2731,7 @@ main(int argc, char **argv)
 				}
 				break;
 			case 'M':
+				benchmarking_option_set = true;
 				if (num_files > 0)
 				{
 					fprintf(stderr, "query mode (-M) should be specifiled before transaction scripts (-f)\n");
@@ -2731,6 +2747,7 @@ main(int argc, char **argv)
 				}
 				break;
 			case 'P':
+				benchmarking_option_set = true;
 				progress = atoi(optarg);
 				if (progress <= 0)
 				{
@@ -2745,6 +2762,8 @@ main(int argc, char **argv)
 					/* get a double from the beginning of option value */
 					double		throttle_value = atof(optarg);
 
+					benchmarking_option_set = true;
+
 					if (throttle_value <= 0.0)
 					{
 						fprintf(stderr, "invalid rate limit: %s\n", optarg);
@@ -2756,14 +2775,19 @@ main(int argc, char **argv)
 				break;
 			case 0:
 				/* This covers long options which take no argument. */
+				if (foreign_keys || unlogged_tables)
+					initializing_option_set = true;
 				break;
 			case 2:				/* tablespace */
+				initializing_option_set = true;
 				tablespace = pg_strdup(optarg);
 				break;
 			case 3:				/* index-tablespace */
+				initializing_option_set = true;
 				index_tablespace = pg_strdup(optarg);
 				break;
 			case 4:
+				benchmarking_option_set = true;
 				sample_rate = atof(optarg);
 				if (sample_rate <= 0.0 || sample_rate > 1.0)
 				{
@@ -2776,6 +2800,7 @@ main(int argc, char **argv)
 				fprintf(stderr, "--aggregate-interval is not currently supported on Windows");
 				exit(1);
 #else
+				benchmarking_option_set = true;
 				agg_interval = atoi(optarg);
 				if (agg_interval <= 0)
 				{
@@ -2808,9 +2833,23 @@ main(int argc, char **argv)
 
 	if (is_init_mode)
 	{
+		if (benchmarking_option_set)
+		{
+			fprintf(stderr, "some parameters cannot be used in initializing mode\n");
+			exit(1);
+		}
+
 		init(is_no_vacuum);
 		exit(0);
 	}
+	else
+	{
+		if (initializing_option_set)
+		{
+			fprintf(stderr, "some parameters cannot be used in benchmarking mode\n");
+			exit(1);
+		}
+	}
 
 	/* Use DEFAULT_NXACTS if neither nxacts nor duration is specified. */
 	if (nxacts <= 0 && duration <= 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