Bonjour Michaƫl,
[...] 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?
Hmmm.Your point is to store the gset/aset status into the meta field, even if the command type is SQL. This is not done for gset, which relies on the non-null prefix, and breaks the assumption that meta is set to something only when the command is a meta command. Why not. I updated the comment, so now meta is none/gset/aset when command type is sql, and I removed the aset field.
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.
I tried to do that.
- * 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.
It is now 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.
Added (I think, if I understood what you suggested.).
- } while (res); + } while (res != NULL); Useless diff.
Yep. Attached an updated v5. -- Fabien.
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> diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index b8864c6ae5..74aadeade0 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 */ @@ -504,14 +505,17 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"}; * not applied. * first_line A short, single-line extract of 'lines', for error reporting. * type SQL_COMMAND or META_COMMAND - * meta The type of meta-command, or META_NONE if command is SQL + * meta The type of meta-command, if command is SQL META_NONE, + * META_GSET or META_ASET which dictate what to do with the + * SQL query result. * argc Number of arguments of the command, 0 if not yet processed. * 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. + * aset do gset on all possible queries of a combined query (\;). * expr Parsed expression, if needed. * stats Time spent in this command. */ @@ -2489,6 +2493,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,17 +2717,20 @@ 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 (META_GSET) or *all* commands (META_ASET). * * Returns true if everything is A-OK, false if any error occurs. */ static bool -readCommandResponse(CState *st, char *varprefix) +readCommandResponse(CState *st, MetaCommand meta, char *varprefix) { PGresult *res; PGresult *next_res; int qrynum = 0; + Assert((meta == META_NONE && varprefix == NULL) || + ((meta == META_GSET || meta == META_ASET) && varprefix != NULL)); + res = PQgetResult(st->con); while (res != NULL) @@ -2736,7 +2745,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 && meta == META_GSET) { 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 +2754,22 @@ readCommandResponse(CState *st, char *varprefix) break; case PGRES_TUPLES_OK: - if (is_last && varprefix != NULL) + if ((is_last && meta == META_GSET) || meta == META_ASET) { - if (PQntuples(res) != 1) + int ntuples = PQntuples(res); + + if (meta == META_GSET && 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; } + /* coldly skip empty result under \aset */ + else if (meta == META_ASET && ntuples <= 0) + break; - /* store results into variables */ + /* store (last/only) row into possibly prefixed variables */ for (int fld = 0; fld < PQnfields(res); fld++) { char *varname = PQfname(res, fld); @@ -2763,9 +2778,9 @@ 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 +3196,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]->meta, + sql_script[st->use_file].commands[st->command]->varprefix)) st->state = CSTATE_END_COMMAND; else st->state = CSTATE_ABORTED; @@ -4660,7 +4677,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 +4821,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; @@ -4830,6 +4847,9 @@ ParseScript(const char *script, const char *desc, int weight) else cmd->varprefix = pg_strdup(command->argv[1]); + /* update the sql command meta */ + 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) } });