pgbench: Remove \cset
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Mon, 25 Mar 2019 15:16:07 +0000 (12:16 -0300)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Mon, 25 Mar 2019 15:16:07 +0000 (12:16 -0300)
Partial revert of commit 6260cc550b0e, "pgbench: add \cset and \gset
commands".

While \gset is widely considered a useful and necessary tool for user-
defined benchmarks, \cset does not have as much value, and its
implementation was considered "not to be up to project standards"
(though I, รlvaro, can't quite understand exactly how).  Therefore,
remove \cset.

Author: Fabien Coelho
Discussion: https://postgr.es/m/alpine.DEB.2.21.1903230716030.18811@lancre
Discussion: https://postgr.es/m/201901101900.mv7zduch6sad@alvherre.pgsql

doc/src/sgml/ref/pgbench.sgml
src/bin/pgbench/pgbench.c
src/bin/pgbench/t/001_pgbench_with_server.pl
src/fe_utils/psqlscan.l
src/include/fe_utils/psqlscan.h
src/include/fe_utils/psqlscan_int.h

index 24833f46bcf56a39b30ff3fe6bf514bb68fd48b9..f11d36620d65a92538812640ecd739bf9e07b1f6 100644 (file)
@@ -963,48 +963,6 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
   </para>
 
   <variablelist>
-   <varlistentry id='pgbench-metacommand-cset'>
-    <term>
-     <literal>\cset [<replaceable>prefix</replaceable>]</literal>
-    </term>
-
-    <listitem>
-     <para>
-      This command may be used to end SQL queries, replacing an embedded
-      semicolon (<literal>\;</literal>) within a compound SQL command.
-     </para>
-
-     <para>
-      When this command is used, the preceding SQL query is expected to
-      return one row, the columns of which are stored into variables named after
-      column names, and prefixed with <replaceable>prefix</replaceable> if provided.
-     </para>
-
-     <para>
-      The following example sends four queries as one compound SQL command,
-      inducing one message sent at the protocol level.
-      The result of the first query is stored into variable <replaceable>one</replaceable>,
-      the results of the third query are stored into variables <replaceable>z_three</replaceable>
-      and <replaceable>z_four</replaceable>,
-      whereas the results of the other queries are discarded.
-<programlisting>
--- compound of four queries
-SELECT 1 AS one \cset
-SELECT 2 AS two \;
-SELECT 3 AS three, 4 AS four \cset z_
-SELECT 5;
-</programlisting>
-     </para>
-
-     <note>
-      <para>
-        <literal>\cset</literal> does not work when empty SQL queries appear
-        within a compound SQL command.
-      </para>
-     </note>
-    </listitem>
-   </varlistentry>
-
    <varlistentry id='pgbench-metacommand-gset'>
     <term>
      <literal>\gset [<replaceable>prefix</replaceable>]</literal>
@@ -1012,8 +970,8 @@ SELECT 5;
 
     <listitem>
      <para>
-      This command may be used to end SQL queries, replacing a final semicolon
-      (<literal>;</literal>).
+      This command may be used to end SQL queries, taking the place of the
+      terminating semicolon (<literal>;</literal>).
      </para>
 
      <para>
@@ -1026,7 +984,7 @@ SELECT 5;
       The following example puts the final account balance from the first query
       into variable <replaceable>abalance</replaceable>, and fills variables
       <replaceable>p_two</replaceable> and <replaceable>p_three</replaceable>
-      with integers from the last query.
+      with integers from the third query.
       The result of the second query is discarded.
 <programlisting>
 UPDATE pgbench_accounts
@@ -1038,13 +996,6 @@ SELECT 1 \;
 SELECT 2 AS two, 3 AS three \gset p_
 </programlisting>
      </para>
-
-     <note>
-      <para>
-        <literal>\gset</literal> does not work when empty SQL queries appear
-        within a compound SQL command.
-      </para>
-     </note>
     </listitem>
    </varlistentry>
 
index 4789ab92eec4bf757958ae6c0d1068bb3bb73683..dde058729db63c50e0c5405f236c73cc8d23836f 100644 (file)
@@ -490,7 +490,6 @@ typedef enum MetaCommand
        META_SETSHELL,                          /* \setshell */
        META_SHELL,                                     /* \shell */
        META_SLEEP,                                     /* \sleep */
-       META_CSET,                                      /* \cset */
        META_GSET,                                      /* \gset */
        META_IF,                                        /* \if */
        META_ELIF,                                      /* \elif */
@@ -521,11 +520,9 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
  * argv                        Command arguments, the first of which is the command or SQL
  *                             string itself.  For SQL commands, after post-processing
  *                             argv[0] is the same as 'lines' with variables substituted.
- * nqueries            In a multi-command SQL line, the number of queries.
- * varprefix   SQL commands terminated with \gset or \cset have this set
+ * varprefix   SQL commands terminated with \gset have this set
  *                             to a non NULL value.  If nonempty, it's used to prefix the
  *                             variable name that receives the value.
- * varprefix_max Allocated size of the varprefix array.
  * expr                        Parsed expression, if needed.
  * stats               Time spent in this command.
  */
@@ -537,9 +534,7 @@ typedef struct Command
        MetaCommand meta;
        int                     argc;
        char       *argv[MAX_ARGS];
-       int                     nqueries;
-       char      **varprefix;
-       int                     varprefix_max;
+       char       *varprefix;
        PgBenchExpr *expr;
        SimpleStats stats;
 } Command;
@@ -619,7 +614,6 @@ static void doLog(TState *thread, CState *st,
 static void processXactStats(TState *thread, CState *st, instr_time *now,
                                 bool skipped, StatsData *agg);
 static void pgbench_error(const char *fmt,...) pg_attribute_printf(1, 2);
-static void allocate_command_varprefix(Command *cmd, int totalqueries);
 static void addScript(ParsedScript script);
 static void *threadRun(void *arg);
 static void finishCon(CState *st);
@@ -2614,8 +2608,6 @@ getMetaCommand(const char *cmd)
                mc = META_ELSE;
        else if (pg_strcasecmp(cmd, "endif") == 0)
                mc = META_ENDIF;
-       else if (pg_strcasecmp(cmd, "cset") == 0)
-               mc = META_CSET;
        else if (pg_strcasecmp(cmd, "gset") == 0)
                mc = META_GSET;
        else
@@ -2848,24 +2840,30 @@ sendCommand(CState *st, Command *command)
 /*
  * Process query response from the backend.
  *
- * If varprefix is not NULL, it's the array of variable prefix names where to
- * store the results.
+ * If varprefix is not NULL, it's the variable name prefix where to store
+ * the results of the *last* command.
  *
  * Returns true if everything is A-OK, false if any error occurs.
  */
 static bool
-readCommandResponse(CState *st, char **varprefix)
+readCommandResponse(CState *st, char *varprefix)
 {
        PGresult   *res;
        int                     qrynum = 0;
 
-       while ((res = PQgetResult(st->con)) != NULL)
+       res = PQgetResult(st->con);
+
+       while (res != NULL)
        {
+               /* look now at the next result to know whether it is the last */
+               PGresult        *next_res = PQgetResult(st->con);
+               bool is_last = (next_res == NULL);
+
                switch (PQresultStatus(res))
                {
                        case PGRES_COMMAND_OK:  /* non-SELECT commands */
                        case PGRES_EMPTY_QUERY: /* may be used for testing no-op overhead */
-                               if (varprefix && varprefix[qrynum] != NULL)
+                               if (is_last && varprefix != NULL)
                                {
                                        fprintf(stderr,
                                                        "client %d script %d command %d query %d: expected one row, got %d\n",
@@ -2876,7 +2874,7 @@ readCommandResponse(CState *st, char **varprefix)
                                break;
 
                        case PGRES_TUPLES_OK:
-                               if (varprefix && varprefix[qrynum] != NULL)
+                               if (is_last && varprefix != NULL)
                                {
                                        if (PQntuples(res) != 1)
                                        {
@@ -2895,9 +2893,8 @@ readCommandResponse(CState *st, char **varprefix)
                                                char       *varname = PQfname(res, fld);
 
                                                /* allocate varname only if necessary, freed below */
-                                               if (*varprefix[qrynum] != '\0')
-                                                       varname =
-                                                               psprintf("%s%s", varprefix[qrynum], varname);
+                                               if (*varprefix != '\0')
+                                                       varname = psprintf("%s%s", varprefix, varname);
 
                                                /* store result as a string */
                                                if (!putVariable(st, "gset", varname,
@@ -2914,7 +2911,7 @@ readCommandResponse(CState *st, char **varprefix)
                                                        return false;
                                                }
 
-                                               if (*varprefix[qrynum] != '\0')
+                                               if (*varprefix != '\0')
                                                        pg_free(varname);
                                        }
                                }
@@ -2935,6 +2932,7 @@ readCommandResponse(CState *st, char **varprefix)
 
                PQclear(res);
                qrynum++;
+               res = next_res;
        }
 
        if (qrynum == 0)
@@ -4248,7 +4246,7 @@ skip_sql_comments(char *sql_command)
  * struct.
  */
 static Command *
-create_sql_command(PQExpBuffer buf, const char *source, int numqueries)
+create_sql_command(PQExpBuffer buf, const char *source)
 {
        Command    *my_command;
        char       *p = skip_sql_comments(buf->data);
@@ -4265,9 +4263,7 @@ create_sql_command(PQExpBuffer buf, const char *source, int numqueries)
        my_command->meta = META_NONE;
        my_command->argc = 0;
        memset(my_command->argv, 0, sizeof(my_command->argv));
-       my_command->nqueries = numqueries;
        my_command->varprefix = NULL;   /* allocated later, if needed */
-       my_command->varprefix_max = 0;
        my_command->expr = NULL;
        initSimpleStats(&my_command->stats);
 
@@ -4284,39 +4280,14 @@ free_command(Command *command)
        for (int i = 0; i < command->argc; i++)
                pg_free(command->argv[i]);
        if (command->varprefix)
-       {
-               for (int i = 0; i < command->varprefix_max; i++)
-                       if (command->varprefix[i] &&
-                               command->varprefix[i][0] != '\0')       /* see ParseScript */
-                               pg_free(command->varprefix[i]);
                pg_free(command->varprefix);
-       }
        /*
         * It should also free expr recursively, but this is currently not needed
-        * as only \{g,c}set commands (which do not have an expression) are freed.
+        * as only gset commands (which do not have an expression) are freed.
         */
        pg_free(command);
 }
 
-/*
- * append "more" text to current compound command which had been interrupted
- * by \cset.
- */
-static void
-append_sql_command(Command *my_command, char *more, int numqueries)
-{
-       Assert(my_command->type == SQL_COMMAND && my_command->lines.len > 0);
-
-       more = skip_sql_comments(more);
-       if (more == NULL)
-               return;
-
-       /* append command text, embedding a ';' in place of the \cset */
-       appendPQExpBuffer(&my_command->lines, ";\n%s", more);
-
-       my_command->nqueries += numqueries;
-}
-
 /*
  * Once an SQL command is fully parsed, possibly by accumulating several
  * parts, complete other fields of the Command structure.
@@ -4350,35 +4321,6 @@ postprocess_sql_command(Command *my_command)
        }
 }
 
-/*
- * Determine the command's varprefix size needed and allocate memory for it
- */
-static void
-allocate_command_varprefix(Command *cmd, int totalqueries)
-{
-       int                     new_max;
-
-       /* sufficient space already allocated? */
-       if (totalqueries <= cmd->varprefix_max)
-               return;
-
-       /* determine the new array size */
-       new_max = Max(cmd->varprefix_max, 2);
-       while (new_max < totalqueries)
-               new_max *= 2;
-
-       /* enlarge the array, zero-initializing the allocated space */
-       if (cmd->varprefix == NULL)
-               cmd->varprefix = pg_malloc0(sizeof(char *) * new_max);
-       else
-       {
-               cmd->varprefix = pg_realloc(cmd->varprefix, sizeof(char *) * new_max);
-               memset(cmd->varprefix + cmd->varprefix_max, 0,
-                          sizeof(char *) * (new_max - cmd->varprefix_max));
-       }
-       cmd->varprefix_max = new_max;
-}
-
 /*
  * Parse a backslash command; return a Command struct, or NULL if comment
  *
@@ -4549,7 +4491,7 @@ process_backslash_command(PsqlScanState sstate, const char *source)
                        syntax_error(source, lineno, my_command->first_line, my_command->argv[0],
                                                 "unexpected argument", NULL, -1);
        }
-       else if (my_command->meta == META_CSET || my_command->meta == META_GSET)
+       else if (my_command->meta == META_GSET)
        {
                if (my_command->argc > 2)
                        syntax_error(source, lineno, my_command->first_line, my_command->argv[0],
@@ -4637,7 +4579,6 @@ ParseScript(const char *script, const char *desc, int weight)
        PQExpBufferData line_buf;
        int                     alloc_num;
        int                     index;
-       bool            saw_cset = false;
        int                     lineno;
        int                     start_offset;
 
@@ -4674,31 +4615,18 @@ ParseScript(const char *script, const char *desc, int weight)
                PsqlScanResult sr;
                promptStatus_t prompt;
                Command    *command = NULL;
-               int                     semicolons;
 
                resetPQExpBuffer(&line_buf);
                lineno = expr_scanner_get_lineno(sstate, start_offset);
 
                sr = psql_scan(sstate, &line_buf, &prompt);
 
-               semicolons = psql_scan_get_escaped_semicolons(sstate);
-
-               if (saw_cset)
-               {
-                       /* the previous multi-line command ended with \cset */
-                       append_sql_command(ps.commands[index - 1], line_buf.data,
-                                                          semicolons + 1);
-                       saw_cset = false;
-               }
-               else
-               {
-                       /* If we collected a new SQL command, process that */
-                       command = create_sql_command(&line_buf, desc, semicolons + 1);
+               /* If we collected a new SQL command, process that */
+               command = create_sql_command(&line_buf, desc);
 
-                       /* store new command */
-                       if (command)
-                               ps.commands[index++] = command;
-               }
+               /* store new command */
+               if (command)
+                       ps.commands[index++] = command;
 
                /* If we reached a backslash, process that */
                if (sr == PSCAN_BACKSLASH)
@@ -4708,56 +4636,31 @@ ParseScript(const char *script, const char *desc, int weight)
                        if (command)
                        {
                                /*
-                                * If this is gset/cset, merge into the preceding command. (We
+                                * If this is gset, merge into the preceding command. (We
                                 * don't use a command slot in this case).
                                 */
-                               if (command->meta == META_CSET ||
-                                       command->meta == META_GSET)
+                               if (command->meta == META_GSET)
                                {
-                                       int                     cindex;
                                        Command    *cmd;
 
-                                       /*
-                                        * If \cset is seen, set flag to leave the command pending
-                                        * for the next iteration to process.
-                                        */
-                                       saw_cset = command->meta == META_CSET;
-
                                        if (index == 0)
                                                syntax_error(desc, lineno, NULL, NULL,
-                                                                        "\\gset/cset cannot start a script",
+                                                                        "\\gset must follow a SQL command",
                                                                         NULL, -1);
 
                                        cmd = ps.commands[index - 1];
 
-                                       if (cmd->type != SQL_COMMAND)
+                                       if (cmd->type != SQL_COMMAND ||
+                                               cmd->varprefix != NULL)
                                                syntax_error(desc, lineno, NULL, NULL,
-                                                                        "\\gset/cset must follow a SQL command",
+                                                                        "\\gset must follow a SQL command",
                                                                         cmd->first_line, -1);
 
-                                       /* this {g,c}set applies to the previous query */
-                                       cindex = cmd->nqueries - 1;
-
-                                       /*
-                                        * now that we know there's a {g,c}set in this command,
-                                        * allocate space for the variable name prefix array.
-                                        */
-                                       allocate_command_varprefix(cmd, cmd->nqueries);
-
-                                       /*
-                                        * Don't allow the previous command be a gset/cset; that
-                                        * would make no sense.
-                                        */
-                                       if (cmd->varprefix[cindex] != NULL)
-                                               syntax_error(desc, lineno, NULL, NULL,
-                                                                        "\\gset/cset cannot follow one another",
-                                                                        NULL, -1);
-
                                        /* get variable prefix */
                                        if (command->argc <= 1 || command->argv[1][0] == '\0')
-                                               cmd->varprefix[cindex] = "";    /* avoid strdup */
+                                               cmd->varprefix = pg_strdup("");
                                        else
-                                               cmd->varprefix[cindex] = pg_strdup(command->argv[1]);
+                                               cmd->varprefix = pg_strdup(command->argv[1]);
 
                                        /* cleanup unused command */
                                        free_command(command);
index 45888dc12e83942b3838b0143528a16936e1fd8e..ad3a7a79f8773d690f9ee93bbec65857ff613ddf 100644 (file)
@@ -538,22 +538,18 @@ pgbench(
 }
        });
 
-# working \gset and \cset
+# working \gset
 pgbench(
        '-t 1', 0,
-       [ qr{type: .*/001_pgbench_gset_and_cset}, qr{processed: 1/1} ],
+       [ qr{type: .*/001_pgbench_gset}, qr{processed: 1/1} ],
        [   qr{command=3.: int 0\b},
                qr{command=5.: int 1\b},
                qr{command=6.: int 2\b},
                qr{command=8.: int 3\b},
-               qr{command=9.: int 4\b},
-               qr{command=10.: int 5\b},
-               qr{command=12.: int 6\b},
-               qr{command=13.: int 7\b},
-               qr{command=14.: int 8\b},
-               qr{command=16.: int 9\b} ],
-       'pgbench gset and cset commands',
-       {   '001_pgbench_gset_and_cset' => q{-- test gset and cset
+               qr{command=10.: int 4\b},
+               qr{command=12.: int 5\b} ],
+       'pgbench gset command',
+       {   '001_pgbench_gset' => q{-- test gset
 -- no columns
 SELECT \gset
 -- one value
@@ -563,21 +559,15 @@ SELECT 0 AS i0 \gset
 SELECT 1 AS i1, 2 AS i2 \gset
 \set i debug(:i1)
 \set i debug(:i2)
--- cset & gset to follow
-SELECT :i2 + 1 AS i3, :i2 * :i2 AS i4 \cset
-  SELECT 5 AS i5 \gset
-\set i debug(:i3)
-\set i debug(:i4)
-\set i debug(:i5)
 -- with prefix
-SELECT 6 AS i6, 7 AS i7 \cset x_
-  SELECT 8 AS i8 \gset y_
-\set i debug(:x_i6)
-\set i debug(:x_i7)
-\set i debug(:y_i8)
+SELECT 3 AS i3 \gset x_
+\set i debug(:x_i3)
 -- overwrite existing variable
-SELECT 0 AS i9, 9 AS i9 \gset
-\set i debug(:i9)
+SELECT 0 AS i4, 4 AS i4 \gset
+\set i debug(:i4)
+-- work on the last SQL command under \;
+\; \; SELECT 0 AS i5 \; SELECT 5 AS i5 \; \; \gset
+\set i debug(:i5)
 } });
 
 # trigger many expression errors
@@ -772,20 +762,17 @@ SELECT LEAST(}.join(', ', (':i') x 256).q{)}
        [   'bad boolean',                     2,
                [qr{malformed variable.*trueXXX}], q{\set b :badtrue or true} ],
 
-       # GSET & CSET
+       # GSET
        [   'gset no row',                    2,
                [qr{expected one row, got 0\b}], q{SELECT WHERE FALSE \gset} ],
-       [   'cset no row',                    2,
-               [qr{expected one row, got 0\b}], q{SELECT WHERE FALSE \cset
-SELECT 1 AS i\gset}, 1 ],
-       [ 'gset alone', 1, [qr{gset/cset cannot start a script}], q{\gset} ],
+       [ 'gset alone', 1, [qr{gset must follow a SQL command}], q{\gset} ],
        [   'gset no SQL',                        1,
-               [qr{gset/cset must follow a SQL command}], q{\set i +1
+               [qr{gset must follow a SQL command}], q{\set i +1
 \gset} ],
        [   'gset too many arguments',                   1,
                [qr{too many arguments}], q{SELECT 1 \gset a b} ],
        [   'gset after gset',                        1,
-           [qr{gset/cset cannot follow one another}], q{SELECT 1 AS i \gset
+           [qr{gset must follow a SQL command}], q{SELECT 1 AS i \gset
 \gset} ],
        [   'gset non SELECT', 2,
                [qr{expected one row, got 0}],
index 321744cddbb1d30dee6cafd79e7912789649647e..b31527b94f4d6bf45df3bdd3b427526ea8244d9f 100644 (file)
@@ -693,15 +693,8 @@ other                      .
         * substitution.  We want these before {self}, also.
         */
 
-"\\";                  {
-                                       /* Count semicolons in compound commands */
-                                       cur_state->escaped_semicolons++;
-                                       /* Force a semicolon into the query buffer */
-                                       psqlscan_emit(cur_state, yytext + 1, 1);
-                               }
-
-"\\":                  {
-                                       /* Force a colon into the query buffer */
+"\\"[;:]               {
+                                       /* Force a semi-colon or colon into the query buffer */
                                        psqlscan_emit(cur_state, yytext + 1, 1);
                                }
 
@@ -1072,9 +1065,6 @@ psql_scan(PsqlScanState state,
        /* Set current output target */
        state->output_buf = query_buf;
 
-       /* Reset number of escaped semicolons seen */
-       state->escaped_semicolons = 0;
-
        /* Set input source */
        if (state->buffer_stack != NULL)
                yy_switch_to_buffer(state->buffer_stack->buf, state->scanner);
@@ -1218,16 +1208,6 @@ psql_scan_reset(PsqlScanState state)
        state->dolqstart = NULL;
 }
 
-/*
- * Return the number of escaped semicolons in the lexed string seen by the
- * previous psql_scan call.
- */
-int
-psql_scan_get_escaped_semicolons(PsqlScanState state)
-{
-       return state->escaped_semicolons;
-}
-
 /*
  * Reselect this lexer (psqlscan.l) after using another one.
  *
index d6fef9ff771fc438d43deecf4e4916259bcf1530..1cf5b2e7fa41e05023251cfb003f1d0eba1ece9f 100644 (file)
@@ -90,8 +90,6 @@ extern PsqlScanResult psql_scan(PsqlScanState state,
 
 extern void psql_scan_reset(PsqlScanState state);
 
-extern int psql_scan_get_escaped_semicolons(PsqlScanState state);
-
 extern void psql_scan_reselect_sql_lexer(PsqlScanState state);
 
 extern bool psql_scan_in_quote(PsqlScanState state);
index 752cc9406a53661c40befe1bf19c1314155fcfa5..42a738f4221fdfa93ac5c4a41c58a56ae770af2f 100644 (file)
@@ -112,7 +112,6 @@ typedef struct PsqlScanStateData
        int                     start_state;    /* yylex's starting/finishing state */
        int                     paren_depth;    /* depth of nesting in parentheses */
        int                     xcdepth;                /* depth of nesting in slash-star comments */
-       int                     escaped_semicolons;     /* number of embedded (\;) semicolons */
        char       *dolqstart;          /* current $foo$ quote start string */
 
        /*