pgbench: Remove \cset
authorAlvaro Herrera <[email protected]>
Mon, 25 Mar 2019 15:16:07 +0000 (12:16 -0300)
committerAlvaro Herrera <[email protected]>
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[email protected]

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 */
 
        /*