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