On Fri, Nov 29, 2019 at 10:34:05AM +0100, Fabien COELHO wrote: >> It seems to me that there is no point to have the variable aset in >> Command because this structure includes already MetaCommand, so the >> information is duplicated. [...] Perhaps I am missing something? > > Yep. ISTM that you are missing that aset is not an independent meta command > like most others but really changes the state of the previous SQL command, > so that it needs to be stored into that with some additional fields. This is > the same with "gset" which is tagged by a non-null "varprefix". > > So I cannot remove the "aset" field.
Still sounds strange to me to invent a new variable to this structure if it is possible to track the exact same thing with an existing part of a Command, or it would make sense to split Command into two different structures with an extra structure used after the parsing for clarity? >> And I would suggest to change readCommandResponse() to use a MetaCommand >> in argument. > > MetaCommand is not enough: we need varprefix, and then distinguishing > between aset and gset. Although this last point can be done with a > MetaCommand, ISTM that a bool is_aset is clear and good enough. It is > possible to switch if you insist on it, but I do not think it is desirable. > > Attached v4 removes an unwanted rebased comment duplication and does minor > changes while re-reading the code. Well, it still looks cleaner to me to just assign the meta field properly within ParseScript(), and you get the same result. And it is also possible to use "meta" to do more sanity checks around META_GSET for some code paths. So I'd actually find the addition of a new argument using a meta command within readCommandResponse() cleaner. - * varprefix SQL commands terminated with \gset have this set + * varprefix SQL commands terminated with \gset or \aset have this set Nit from v4: varprefix can be used for \gset and \aset, and the comment was not updated. + /* coldly skip empty result under \aset */ + if (ntuples <= 0) + break; Shouldn't this check after \aset? And it seems to me that this code path is not taken, so a test would be nice. - } while (res); + } while (res != NULL); Useless diff. The conflicts came from the switch to the common logging of src/common/. That was no big deal to solve. -- Michael
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index b8864c6ae5..ebb4eb2c5c 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -480,6 +480,7 @@ typedef enum MetaCommand
META_SHELL, /* \shell */
META_SLEEP, /* \sleep */
META_GSET, /* \gset */
+ META_ASET, /* \aset */
META_IF, /* \if */
META_ELIF, /* \elif */
META_ELSE, /* \else */
@@ -509,7 +510,7 @@ 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.
- * varprefix SQL commands terminated with \gset have this set
+ * varprefix SQL commands terminated with \gset or \aset have this set
* to a non NULL value. If nonempty, it's used to prefix the
* variable name that receives the value.
* expr Parsed expression, if needed.
@@ -2489,6 +2490,8 @@ getMetaCommand(const char *cmd)
mc = META_ENDIF;
else if (pg_strcasecmp(cmd, "gset") == 0)
mc = META_GSET;
+ else if (pg_strcasecmp(cmd, "aset") == 0)
+ mc = META_ASET;
else
mc = META_NONE;
return mc;
@@ -2711,12 +2714,12 @@ sendCommand(CState *st, Command *command)
* Process query response from the backend.
*
* If varprefix is not NULL, it's the variable name prefix where to store
- * the results of the *last* command.
+ * the results of the *last* command (gset) or *all* commands (aset).
*
* Returns true if everything is A-OK, false if any error occurs.
*/
static bool
-readCommandResponse(CState *st, char *varprefix)
+readCommandResponse(CState *st, char *varprefix, MetaCommand meta)
{
PGresult *res;
PGresult *next_res;
@@ -2736,7 +2739,7 @@ readCommandResponse(CState *st, char *varprefix)
{
case PGRES_COMMAND_OK: /* non-SELECT commands */
case PGRES_EMPTY_QUERY: /* may be used for testing no-op overhead */
- if (is_last && varprefix != NULL)
+ if (is_last && varprefix != NULL && meta != META_ASET)
{
pg_log_error("client %d script %d command %d query %d: expected one row, got %d",
st->id, st->use_file, st->command, qrynum, 0);
@@ -2745,16 +2748,26 @@ readCommandResponse(CState *st, char *varprefix)
break;
case PGRES_TUPLES_OK:
- if (is_last && varprefix != NULL)
+ if (varprefix != NULL && (is_last || meta == META_ASET))
{
- if (PQntuples(res) != 1)
+ int ntuples = PQntuples(res);
+
+ if (meta != META_ASET && ntuples != 1)
{
+ /* under \gset, report the error */
pg_log_error("client %d script %d command %d query %d: expected one row, got %d",
st->id, st->use_file, st->command, qrynum, PQntuples(res));
goto error;
}
- /* store results into variables */
+ /* coldly skip empty result under \aset */
+ if (ntuples <= 0)
+ {
+ Assert(meta == META_ASET);
+ break;
+ }
+
+ /* store results into possibly prefixed variables */
for (int fld = 0; fld < PQnfields(res); fld++)
{
char *varname = PQfname(res, fld);
@@ -2763,9 +2776,10 @@ readCommandResponse(CState *st, char *varprefix)
if (*varprefix != '\0')
varname = psprintf("%s%s", varprefix, varname);
- /* store result as a string */
- if (!putVariable(st, "gset", varname,
- PQgetvalue(res, 0, fld)))
+ /* store last row result as a string */
+ if (!putVariable(st, meta == META_ASET ? "aset" : "gset",
+ varname,
+ PQgetvalue(res, ntuples - 1, fld)))
{
/* internal error */
pg_log_error("client %d script %d command %d query %d: error storing into variable %s",
@@ -3181,7 +3195,9 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
return; /* don't have the whole result yet */
/* store or discard the query results */
- if (readCommandResponse(st, sql_script[st->use_file].commands[st->command]->varprefix))
+ if (readCommandResponse(st,
+ sql_script[st->use_file].commands[st->command]->varprefix,
+ sql_script[st->use_file].commands[st->command]->meta))
st->state = CSTATE_END_COMMAND;
else
st->state = CSTATE_ABORTED;
@@ -4660,7 +4676,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_GSET)
+ else if (my_command->meta == META_GSET || my_command->meta == META_ASET)
{
if (my_command->argc > 2)
syntax_error(source, lineno, my_command->first_line, my_command->argv[0],
@@ -4804,10 +4820,10 @@ ParseScript(const char *script, const char *desc, int weight)
if (command)
{
/*
- * If this is gset, merge into the preceding command. (We
- * don't use a command slot in this case).
+ * If this is gset or aset, merge into the preceding command.
+ * (We don't use a command slot in this case).
*/
- if (command->meta == META_GSET)
+ if (command->meta == META_GSET || command->meta == META_ASET)
{
Command *cmd;
@@ -4829,6 +4845,7 @@ ParseScript(const char *script, const char *desc, int weight)
cmd->varprefix = pg_strdup("");
else
cmd->varprefix = pg_strdup(command->argv[1]);
+ cmd->meta = command->meta;
/* cleanup unused command */
free_command(command);
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 25ea17f7d1..04ab1859f0 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -591,7 +591,7 @@ pgbench(
}
});
-# working \gset
+# working \gset and \aset
pgbench(
'-t 1', 0,
[ qr{type: .*/001_pgbench_gset}, qr{processed: 1/1} ],
@@ -601,9 +601,11 @@ pgbench(
qr{command=6.: int 2\b},
qr{command=8.: int 3\b},
qr{command=10.: int 4\b},
- qr{command=12.: int 5\b}
+ qr{command=12.: int 5\b},
+ qr{command=15.: int 8\b},
+ qr{command=16.: int 7\b}
],
- 'pgbench gset command',
+ 'pgbench gset & aset commands',
{
'001_pgbench_gset' => q{-- test gset
-- no columns
@@ -624,6 +626,12 @@ SELECT 0 AS i4, 4 AS i4 \gset
-- work on the last SQL command under \;
\; \; SELECT 0 AS i5 \; SELECT 5 AS i5 \; \; \gset
\set i debug(:i5)
+-- test aset, which applies to a combined query
+\; SELECT 6 AS i6 \; SELECT 7 AS i7 \; \aset
+-- unless it returns more than one row, last is kept
+SELECT 8 AS i6 UNION SELECT 9 ORDER BY 1 DESC \aset
+\set i debug(:i6)
+\set i debug(:i7)
}
});
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 41b3880c91..58a2aa3bf2 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -1057,18 +1057,29 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
<varlistentry id='pgbench-metacommand-gset'>
<term>
<literal>\gset [<replaceable>prefix</replaceable>]</literal>
+ <literal>\aset [<replaceable>prefix</replaceable>]</literal>
</term>
<listitem>
<para>
- This command may be used to end SQL queries, taking the place of the
+ These commands may be used to end SQL queries, taking the place of the
terminating semicolon (<literal>;</literal>).
</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.
+ When the <literal>\gset</literal> 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>
+ When the <literal>\aset</literal> command is used, all combined SQL queries
+ (separated by <literal>\;</literal>) have their columns stored into variables
+ named after column names, and prefixed with <replaceable>prefix</replaceable>
+ if provided. If a query returns no row, no assignment is made and the variable
+ can be tested for existence to detect this. If a query returns more than one
+ row, the last value is kept.
</para>
<para>
@@ -1077,6 +1088,8 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
<replaceable>p_two</replaceable> and <replaceable>p_three</replaceable>
with integers from the third query.
The result of the second query is discarded.
+ The result of the two last combined queries are stored in variables
+ <replaceable>four</replaceable> and <replaceable>five</replaceable>.
<programlisting>
UPDATE pgbench_accounts
SET abalance = abalance + :delta
@@ -1085,6 +1098,7 @@ UPDATE pgbench_accounts
-- compound of two queries
SELECT 1 \;
SELECT 2 AS two, 3 AS three \gset p_
+SELECT 4 AS four \; SELECT 5 AS five \aset
</programlisting>
</para>
</listitem>
signature.asc
Description: PGP signature
