From 79681844297a9d499a10094a17e52b365afdd8bb Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 11 Aug 2017 15:19:40 -0400
Subject: [PATCH] Remove pgbench's restriction on placement of -M switch.

Previously the -M switch had to appear before any switch that directly
or indirectly specified a benchmarking script.  This was both confusing
and inadequately documented, as per gripe from Tatsuo Ishii.  We can
remove the restriction at the cost of making an extra pass over the
lists of SQL commands, which seems like a cheap price (the string scans
themselves likely cost much more).  The change is just to not extract
parameters from the SQL commands until we have finished parsing the
switches and know the final value of -M.

Per discussion, we'll treat this as a low-grade bug fix and sneak it
into v10, rather than holding it for v11.

Tom Lane, reviewed by Tatsuo Ishii and Fabien Coelho

Discussion: https://postgr.es/m/20170802.110328.1963639094551443169.t-ishii@sraoss.co.jp
Discussion: https://postgr.es/m/10208.1502465077@sss.pgh.pa.us
---
 src/bin/pgbench/pgbench.c | 59 ++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 25 deletions(-)

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 4d364a1db4c..ae78c7b1d46 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2844,15 +2844,18 @@ init(bool is_no_vacuum)
 }
 
 /*
- * Parse the raw sql and replace :param to $n.
+ * Replace :param with $n throughout the command's SQL text, which
+ * is a modifiable string in cmd->argv[0].
  */
 static bool
-parseQuery(Command *cmd, const char *raw_sql)
+parseQuery(Command *cmd)
 {
 	char	   *sql,
 			   *p;
 
-	sql = pg_strdup(raw_sql);
+	/* We don't want to scribble on cmd->argv[0] until done */
+	sql = pg_strdup(cmd->argv[0]);
+
 	cmd->argc = 1;
 
 	p = sql;
@@ -2874,7 +2877,8 @@ parseQuery(Command *cmd, const char *raw_sql)
 
 		if (cmd->argc >= MAX_ARGS)
 		{
-			fprintf(stderr, "statement has too many arguments (maximum is %d): %s\n", MAX_ARGS - 1, raw_sql);
+			fprintf(stderr, "statement has too many arguments (maximum is %d): %s\n",
+					MAX_ARGS - 1, cmd->argv[0]);
 			pg_free(name);
 			return false;
 		}
@@ -2886,6 +2890,7 @@ parseQuery(Command *cmd, const char *raw_sql)
 		cmd->argc++;
 	}
 
+	pg_free(cmd->argv[0]);
 	cmd->argv[0] = sql;
 	return true;
 }
@@ -2983,9 +2988,15 @@ process_sql_command(PQExpBuffer buf, const char *source)
 	my_command = (Command *) pg_malloc0(sizeof(Command));
 	my_command->command_num = num_commands++;
 	my_command->type = SQL_COMMAND;
-	my_command->argc = 0;
 	initSimpleStats(&my_command->stats);
 
+	/*
+	 * Install query text as the sole argv string.  If we are using a
+	 * non-simple query mode, we'll extract parameters from it later.
+	 */
+	my_command->argv[0] = pg_strdup(p);
+	my_command->argc = 1;
+
 	/*
 	 * If SQL command is multi-line, we only want to save the first line as
 	 * the "line" label.
@@ -3000,21 +3011,6 @@ process_sql_command(PQExpBuffer buf, const char *source)
 	else
 		my_command->line = pg_strdup(p);
 
-	switch (querymode)
-	{
-		case QUERY_SIMPLE:
-			my_command->argv[0] = pg_strdup(p);
-			my_command->argc++;
-			break;
-		case QUERY_EXTENDED:
-		case QUERY_PREPARED:
-			if (!parseQuery(my_command, p))
-				exit(1);
-			break;
-		default:
-			exit(1);
-	}
-
 	return my_command;
 }
 
@@ -3896,11 +3892,6 @@ main(int argc, char **argv)
 				break;
 			case 'M':
 				benchmarking_option_set = true;
-				if (num_scripts > 0)
-				{
-					fprintf(stderr, "query mode (-M) should be specified before any transaction scripts (-f or -b)\n");
-					exit(1);
-				}
 				for (querymode = 0; querymode < NUM_QUERYMODE; querymode++)
 					if (strcmp(optarg, QUERYMODE[querymode]) == 0)
 						break;
@@ -4006,6 +3997,24 @@ main(int argc, char **argv)
 		internal_script_used = true;
 	}
 
+	/* if not simple query mode, parse the script(s) to find parameters */
+	if (querymode != QUERY_SIMPLE)
+	{
+		for (i = 0; i < num_scripts; i++)
+		{
+			Command   **commands = sql_script[i].commands;
+			int			j;
+
+			for (j = 0; commands[j] != NULL; j++)
+			{
+				if (commands[j]->type != SQL_COMMAND)
+					continue;
+				if (!parseQuery(commands[j]))
+					exit(1);
+			}
+		}
+	}
+
 	/* compute total_weight */
 	for (i = 0; i < num_scripts; i++)
 		/* cannot overflow: weight is 32b, total_weight 64b */
-- 
2.30.2