pgbench - add \aset to store results of a combined query
Hello devs,
A long time ago I submitted a pgbench \into command to store results of
queries into variables independently of the query being processed, which
got turn into \gset (;) and \cset (\;), which got committed, then \cset
was removed because it was not "up to standard", as it could not work with
empty query (the underlying issue is that pg silently skips empty queries,
so that "\; SELECT 1 \; \; SELECT 3," returns 2 results instead of 4, a
misplaced optimisation from my point of view).
Now there is a pgbench \gset which allows to extract the results of
variables of the last query, but as it does both setting and ending a
query at the same time, there is no way to set variables out of a combined
(\;) query but the last, which is the kind of non orthogonal behavior that
I dislike much.
This annoys me because testing the performance of combined queries cannot
be tested if the script needs to extract variables.
To make the feature somehow accessible to combined queries, the attached
patch adds the "\aset" (all set) command to store all results of queries
which return just one row into variables, i.e.:
SELECT 1 AS one \;
SELECT 2 AS two UNION SELECT 2 \;
SELECT 3 AS three \aset
will set both "one" and "three", while "two" is not set because there were
two rows. It is a kind of more permissive \gset.
Because it does it for all queries, there is no need for synchronizing
with the underlying queries, which made the code for \cset both awkward
and with limitations. Hopefully this version might be "up to standard".
I'll see. I'm in no hurry:-)
--
Fabien.
Attachments:
pgbench-aset-1.patchtext/x-diff; name=pgbench-aset-1.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index ee2501be55..83eb7f5f4a 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -966,18 +966,27 @@ 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>) which return just one row see their
+ columns stored into variables named after column names, and prefixed with
+ <replaceable>prefix</replaceable> if provided. Other queries are ignored.
</para>
<para>
@@ -986,6 +995,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
@@ -994,6 +1005,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 99529de52a..9bae707368 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -461,6 +461,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 */
@@ -493,6 +494,7 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
* 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.
+ * aset do gset on all possible queries of a combined query (\;).
* expr Parsed expression, if needed.
* stats Time spent in this command.
*/
@@ -505,6 +507,7 @@ typedef struct Command
int argc;
char *argv[MAX_ARGS];
char *varprefix;
+ bool aset;
PgBenchExpr *expr;
SimpleStats stats;
} Command;
@@ -2493,6 +2496,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;
@@ -2729,7 +2734,7 @@ sendCommand(CState *st, Command *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, bool is_aset)
{
PGresult *res;
int qrynum = 0;
@@ -2746,7 +2751,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 && !is_aset)
{
fprintf(stderr,
"client %d script %d command %d query %d: expected one row, got %d\n",
@@ -2757,10 +2762,15 @@ readCommandResponse(CState *st, char *varprefix)
break;
case PGRES_TUPLES_OK:
- if (is_last && varprefix != NULL)
+ if (varprefix != NULL && (is_last || is_aset))
{
if (PQntuples(res) != 1)
{
+ /* under \aset, ignore results which do not return one row */
+ if (is_aset)
+ break;
+
+ /* else under \gset, report the error */
fprintf(stderr,
"client %d script %d command %d query %d: expected one row, got %d\n",
st->id, st->use_file, st->command, qrynum, PQntuples(res));
@@ -2780,7 +2790,7 @@ readCommandResponse(CState *st, char *varprefix)
varname = psprintf("%s%s", varprefix, varname);
/* store result as a string */
- if (!putVariable(st, "gset", varname,
+ if (!putVariable(st, is_aset ? "aset" : "gset", varname,
PQgetvalue(res, 0, fld)))
{
/* internal error */
@@ -3199,7 +3209,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]->aset))
st->state = CSTATE_END_COMMAND;
else
st->state = CSTATE_ABORTED;
@@ -4135,6 +4147,7 @@ create_sql_command(PQExpBuffer buf, const char *source)
my_command->argc = 0;
memset(my_command->argv, 0, sizeof(my_command->argv));
my_command->varprefix = NULL; /* allocated later, if needed */
+ my_command->aset = false;
my_command->expr = NULL;
initSimpleStats(&my_command->stats);
@@ -4363,7 +4376,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],
@@ -4508,10 +4521,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;
@@ -4533,6 +4546,7 @@ ParseScript(const char *script, const char *desc, int weight)
cmd->varprefix = pg_strdup("");
else
cmd->varprefix = pg_strdup(command->argv[1]);
+ cmd->aset = command->meta == META_ASET;
/* 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 62906d5e55..c461ff7d5b 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -538,7 +538,7 @@ pgbench(
}
});
-# working \gset
+# working \gset and \aset
pgbench(
'-t 1', 0,
[ qr{type: .*/001_pgbench_gset}, qr{processed: 1/1} ],
@@ -547,8 +547,10 @@ 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} ],
- 'pgbench gset command',
+ qr{command=12.: int 5\b},
+ qr{command=15.: int 6\b},
+ qr{command=16.: int 7\b} ],
+ 'pgbench gset & aset commands',
{ '001_pgbench_gset' => q{-- test gset
-- no columns
SELECT \gset
@@ -568,6 +570,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
+SELECT 8 AS i6 UNION SELECT 9 \aset
+\set i debug(:i6)
+\set i debug(:i7)
} });
# trigger many expression errors
V2 is a rebase.
A long time ago I submitted a pgbench \into command to store results of
queries into variables independently of the query being processed, which got
turn into \gset (;) and \cset (\;), which got committed, then \cset was
removed because it was not "up to standard", as it could not work with empty
query (the underlying issue is that pg silently skips empty queries, so that
"\; SELECT 1 \; \; SELECT 3," returns 2 results instead of 4, a misplaced
optimisation from my point of view).Now there is a pgbench \gset which allows to extract the results of variables
of the last query, but as it does both setting and ending a query at the same
time, there is no way to set variables out of a combined (\;) query but the
last, which is the kind of non orthogonal behavior that I dislike much.This annoys me because testing the performance of combined queries cannot be
tested if the script needs to extract variables.To make the feature somehow accessible to combined queries, the attached
patch adds the "\aset" (all set) command to store all results of queries
which return just one row into variables, i.e.:SELECT 1 AS one \;
SELECT 2 AS two UNION SELECT 2 \;
SELECT 3 AS three \asetwill set both "one" and "three", while "two" is not set because there were
two rows. It is a kind of more permissive \gset.Because it does it for all queries, there is no need for synchronizing with
the underlying queries, which made the code for \cset both awkward and with
limitations. Hopefully this version might be "up to standard".
I'll see. I'm in no hurry:-)
--
Fabien.
Attachments:
pgbench-aset-2.patchtext/x-diff; name=pgbench-aset-2.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index ef22a484e7..200f6b0010 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -966,18 +966,27 @@ 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>) which return just one row see their
+ columns stored into variables named after column names, and prefixed with
+ <replaceable>prefix</replaceable> if provided. Other queries are ignored.
</para>
<para>
@@ -986,6 +995,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
@@ -994,6 +1005,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 8b84658ccd..6895187455 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -461,6 +461,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 */
@@ -493,6 +494,7 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
* 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.
+ * aset do gset on all possible queries of a combined query (\;).
* expr Parsed expression, if needed.
* stats Time spent in this command.
*/
@@ -505,6 +507,7 @@ typedef struct Command
int argc;
char *argv[MAX_ARGS];
char *varprefix;
+ bool aset;
PgBenchExpr *expr;
SimpleStats stats;
} Command;
@@ -2479,6 +2482,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;
@@ -2715,7 +2720,7 @@ sendCommand(CState *st, Command *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, bool is_aset)
{
PGresult *res;
PGresult *next_res;
@@ -2735,7 +2740,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 && !is_aset)
{
fprintf(stderr,
"client %d script %d command %d query %d: expected one row, got %d\n",
@@ -2745,10 +2750,15 @@ readCommandResponse(CState *st, char *varprefix)
break;
case PGRES_TUPLES_OK:
- if (is_last && varprefix != NULL)
+ if (varprefix != NULL && (is_last || is_aset))
{
if (PQntuples(res) != 1)
{
+ /* under \aset, ignore results which do not return one row */
+ if (is_aset)
+ break;
+
+ /* else under \gset, report the error */
fprintf(stderr,
"client %d script %d command %d query %d: expected one row, got %d\n",
st->id, st->use_file, st->command, qrynum, PQntuples(res));
@@ -2765,7 +2775,7 @@ readCommandResponse(CState *st, char *varprefix)
varname = psprintf("%s%s", varprefix, varname);
/* store result as a string */
- if (!putVariable(st, "gset", varname,
+ if (!putVariable(st, is_aset ? "aset" : "gset", varname,
PQgetvalue(res, 0, fld)))
{
/* internal error */
@@ -3189,7 +3199,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]->aset))
st->state = CSTATE_END_COMMAND;
else
st->state = CSTATE_ABORTED;
@@ -4125,6 +4137,7 @@ create_sql_command(PQExpBuffer buf, const char *source)
my_command->argc = 0;
memset(my_command->argv, 0, sizeof(my_command->argv));
my_command->varprefix = NULL; /* allocated later, if needed */
+ my_command->aset = false;
my_command->expr = NULL;
initSimpleStats(&my_command->stats);
@@ -4353,7 +4366,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],
@@ -4498,10 +4511,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;
@@ -4523,6 +4536,7 @@ ParseScript(const char *script, const char *desc, int weight)
cmd->varprefix = pg_strdup("");
else
cmd->varprefix = pg_strdup(command->argv[1]);
+ cmd->aset = (command->meta == META_ASET);
/* 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 dc2c72fa92..87ef7d9136 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -538,7 +538,7 @@ pgbench(
}
});
-# working \gset
+# working \gset and \aset
pgbench(
'-t 1', 0,
[ qr{type: .*/001_pgbench_gset}, qr{processed: 1/1} ],
@@ -548,9 +548,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 6\b},
+ qr{command=16.: int 7\b}
],
- 'pgbench gset command',
+ 'pgbench gset & aset commands',
{
'001_pgbench_gset' => q{-- test gset
-- no columns
@@ -571,6 +573,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
+SELECT 8 AS i6 UNION SELECT 9 \aset
+\set i debug(:i6)
+\set i debug(:i7)
}
});
SELECT 1 AS one \;
SELECT 2 AS two UNION SELECT 2 \;
SELECT 3 AS three \asetwill set both "one" and "three", while "two" is not set because there were
two rows. It is a kind of more permissive \gset.
Are you sure two is not set :)?
SELECT 2 AS two UNION SELECT 2; -- only returns one row.
but
SELECT 2 AS two UNION SELECT 10; -- returns the two rows.
Is this the expected behavior with \aset? In my opinion throwing a valid error like "client 0 script 0 command 0 query 0: expected one row, got 2" make more sense.
- With \gset
SELECT 2 AS two UNION SELECT 10 \gset
INSERT INTO test VALUES(:two,0,0);
$ pgbench postgres -f pgbench_aset.sql -T 1 -j 1 -c 1 -s 10
starting vacuum...end.
client 0 script 0 command 0 query 0: expected one row, got 2
transaction type: pgbench_aset.sql
scaling factor: 10
query mode: simple
number of clients: 1
number of threads: 1
duration: 1 s
number of transactions actually processed: 0
Run was aborted; the above results are incomplete.
- With \aset
SELECT 2 AS two UNION SELECT 10 \aset
INSERT INTO test VALUES(:two,0,0);
vagrant@vagrant:~/percona/postgresql$ pgbench postgres -f pgbench_aset.sql -T 1 -j 1 -c 1 -s 10
starting vacuum...end.
client 0 script 0 aborted in command 1 query 0: ERROR: syntax error at or near ":"
LINE 1: INSERT INTO test VALUES(:two,0,0);
^
transaction type: pgbench_aset.sql
scaling factor: 10
query mode: simple
number of clients: 1
number of threads: 1
duration: 1 s
number of transactions actually processed: 0
Run was aborted; the above results are incomplete.
The new status of this patch is: Waiting on Author
Hello Ibrar,
SELECT 1 AS one \;
SELECT 2 AS two UNION SELECT 2 \;
SELECT 3 AS three \asetwill set both "one" and "three", while "two" is not set because there were
two rows. It is a kind of more permissive \gset.Are you sure two is not set :)?
SELECT 2 AS two UNION SELECT 2; -- only returns one row.
but
SELECT 2 AS two UNION SELECT 10; -- returns the two rows.
Indeed, my intension was to show an example like the second.
Is this the expected behavior with \aset?
In my opinion throwing a valid error like "client 0 script 0 command 0
query 0: expected one row, got 2" make more sense.
Hmmm. My intention with \aset is really NOT to throw an error. With
pgbench, the existence of the variable can be tested later to know whether
it was assigned or not, eg:
SELECT 1 AS x \;
-- 2 rows, no assignment
SELECT 'calvin' AS firstname UNION SELECT 'hobbes' \;
SELECT 2 AS z \aset
-- next test is false
\if :{?firstname}
...
\endif
The rational is that one may want to benefit from combined queries (\;)
which result in less communication thus has lower latency, but still be
interested in extracting some results.
The question is what to do if the query returns 0 or >1 rows. If an error
is raised, the construct cannot be used for testing whether there is one
result or not, eg for a query returning 0 or 1 row, you could not write:
\set id random(1, :number_of_users)
SELECT firtname AS fn FROM user WHERE id = :id \aset
\if :{?fn}
-- the user exists, proceed with further queries
\else
-- no user, maybe it was removed, it is not an error
\endif
Another option would to just assign the value so that
- on 0 row no assignment is made, and it can be tested afterwards.
- on >1 rows the last (first?) value is kept. I took last so to
ensure that all results are received.
I think that having some permissive behavior allows to write some more
interesting test scripts that use combined queries and extract values.
What do you think?
- With \gset
SELECT 2 AS two UNION SELECT 10 \gset
INSERT INTO test VALUES(:two,0,0);client 0 script 0 command 0 query 0: expected one row, got 2
Run was aborted; the above results are incomplete.
Yes, that is the intented behavior.
- With \aset
SELECT 2 AS two UNION SELECT 10 \aset
INSERT INTO test VALUES(:two,0,0);
[...]
client 0 script 0 aborted in command 1 query 0: ERROR: syntax error at or near ":"
Indeed, the user should test whether the variable was assigned before
using it if the result is not warranted to return one row.
The new status of this patch is: Waiting on Author
The attached patch implements the altered behavior described above.
--
Fabien.
Attachments:
pgbench-aset-3.patchtext/x-diff; name=pgbench-aset-3.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index e3b73a4cf5..52949d9a35 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -966,18 +966,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>
@@ -986,6 +997,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
@@ -994,6 +1007,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 8b84658ccd..477b6c873b 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -461,6 +461,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 */
@@ -493,6 +494,7 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
* 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.
+ * aset do gset on all possible queries of a combined query (\;).
* expr Parsed expression, if needed.
* stats Time spent in this command.
*/
@@ -505,6 +507,7 @@ typedef struct Command
int argc;
char *argv[MAX_ARGS];
char *varprefix;
+ bool aset;
PgBenchExpr *expr;
SimpleStats stats;
} Command;
@@ -2479,6 +2482,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;
@@ -2710,12 +2715,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, bool is_aset)
{
PGresult *res;
PGresult *next_res;
@@ -2735,7 +2740,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 && !is_aset)
{
fprintf(stderr,
"client %d script %d command %d query %d: expected one row, got %d\n",
@@ -2745,16 +2750,23 @@ readCommandResponse(CState *st, char *varprefix)
break;
case PGRES_TUPLES_OK:
- if (is_last && varprefix != NULL)
+ if (varprefix != NULL && (is_last || is_aset))
{
- if (PQntuples(res) != 1)
+ int ntuples = PQntuples(res);
+
+ if (!is_aset && ntuples != 1)
{
+ /* under \gset, report the error */
fprintf(stderr,
"client %d script %d command %d query %d: expected one row, got %d\n",
st->id, st->use_file, st->command, qrynum, PQntuples(res));
goto error;
}
+ /* skip empty result under \aset */
+ if (ntuples <= 0)
+ break;
+
/* store results into variables */
for (int fld = 0; fld < PQnfields(res); fld++)
{
@@ -2764,15 +2776,14 @@ 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, is_aset ? "aset" : "gset", varname,
+ PQgetvalue(res, ntuples - 1, fld)))
{
/* internal error */
fprintf(stderr,
"client %d script %d command %d query %d: error storing into variable %s\n",
- st->id, st->use_file, st->command, qrynum,
- varname);
+ st->id, st->use_file, st->command, qrynum, varname);
goto error;
}
@@ -3189,7 +3200,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]->aset))
st->state = CSTATE_END_COMMAND;
else
st->state = CSTATE_ABORTED;
@@ -4125,6 +4138,7 @@ create_sql_command(PQExpBuffer buf, const char *source)
my_command->argc = 0;
memset(my_command->argv, 0, sizeof(my_command->argv));
my_command->varprefix = NULL; /* allocated later, if needed */
+ my_command->aset = false;
my_command->expr = NULL;
initSimpleStats(&my_command->stats);
@@ -4353,7 +4367,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],
@@ -4498,10 +4512,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;
@@ -4523,6 +4537,7 @@ ParseScript(const char *script, const char *desc, int weight)
cmd->varprefix = pg_strdup("");
else
cmd->varprefix = pg_strdup(command->argv[1]);
+ cmd->aset = (command->meta == META_ASET);
/* 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 dc2c72fa92..62cc0bb1ac 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -538,7 +538,7 @@ pgbench(
}
});
-# working \gset
+# working \gset and \aset
pgbench(
'-t 1', 0,
[ qr{type: .*/001_pgbench_gset}, qr{processed: 1/1} ],
@@ -548,9 +548,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
@@ -571,6 +573,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)
}
});
On Wed, Jul 10, 2019 at 11:33 AM Fabien COELHO <coelho@cri.ensmp.fr> wrote:
Hello Ibrar,
SELECT 1 AS one \;
SELECT 2 AS two UNION SELECT 2 \;
SELECT 3 AS three \asetwill set both "one" and "three", while "two" is not set because there
were
two rows. It is a kind of more permissive \gset.
Are you sure two is not set :)?
SELECT 2 AS two UNION SELECT 2; -- only returns one row.
but
SELECT 2 AS two UNION SELECT 10; -- returns the two rows.Indeed, my intension was to show an example like the second.
Is this the expected behavior with \aset?
In my opinion throwing a valid error like "client 0 script 0 command 0
query 0: expected one row, got 2" make more sense.Hmmm. My intention with \aset is really NOT to throw an error. With
pgbench, the existence of the variable can be tested later to know whether
it was assigned or not, eg:SELECT 1 AS x \;
-- 2 rows, no assignment
SELECT 'calvin' AS firstname UNION SELECT 'hobbes' \;
SELECT 2 AS z \aset
-- next test is false
\if :{?firstname}
...
\endifThe rational is that one may want to benefit from combined queries (\;)
which result in less communication thus has lower latency, but still be
interested in extracting some results.The question is what to do if the query returns 0 or >1 rows. If an error
is raised, the construct cannot be used for testing whether there is one
result or not, eg for a query returning 0 or 1 row, you could not write:\set id random(1, :number_of_users)
SELECT firtname AS fn FROM user WHERE id = :id \aset
\if :{?fn}
-- the user exists, proceed with further queries
\else
-- no user, maybe it was removed, it is not an error
\endifAnother option would to just assign the value so that
- on 0 row no assignment is made, and it can be tested afterwards.
- on >1 rows the last (first?) value is kept. I took last so to
ensure that all results are received.I think that having some permissive behavior allows to write some more
interesting test scripts that use combined queries and extract values.What do you think?
Yes, I think that make more sense.
- With \gset
SELECT 2 AS two UNION SELECT 10 \gset
INSERT INTO test VALUES(:two,0,0);client 0 script 0 command 0 query 0: expected one row, got 2
Run was aborted; the above results are incomplete.Yes, that is the intented behavior.
- With \aset
SELECT 2 AS two UNION SELECT 10 \aset
INSERT INTO test VALUES(:two,0,0);
[...]
client 0 script 0 aborted in command 1 query 0: ERROR: syntax error ator near ":"
Indeed, the user should test whether the variable was assigned before
using it if the result is not warranted to return one row.The new status of this patch is: Waiting on Author
The attached patch implements the altered behavior described above.
--
Fabien.
--
Ibrar Ahmed
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not tested
The patch passed my review, I have not reviewed the documentation changes.
The new status of this patch is: Ready for Committer
On Thu, Aug 15, 2019 at 06:30:13PM +0000, Ibrar Ahmed wrote:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not testedThe patch passed my review, I have not reviewed the documentation changes.
The new status of this patch is: Ready for Committer
@@ -524,6 +526,7 @@ typedef struct Command
int argc;
char *argv[MAX_ARGS];
char *varprefix;
+ bool aset;
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. And I would suggest to change
readCommandResponse() to use a MetaCommand in argument. Perhaps I am
missing something?
--
Michael
Michaï¿œl,
+ bool aset;
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.
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.
--
Fabien.
Attachments:
pgbench-aset-4.patchtext/x-diff; name=pgbench-aset-4.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 4c48a58ed2..2c1110c054 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -1026,18 +1026,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>
@@ -1046,6 +1057,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
@@ -1054,6 +1067,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 4a7ac1f821..da67846814 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 */
@@ -512,6 +513,7 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
* 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.
+ * aset do gset on all possible queries of a combined query (\;).
* expr Parsed expression, if needed.
* stats Time spent in this command.
*/
@@ -524,6 +526,7 @@ typedef struct Command
int argc;
char *argv[MAX_ARGS];
char *varprefix;
+ bool aset;
PgBenchExpr *expr;
SimpleStats stats;
} Command;
@@ -2503,6 +2506,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;
@@ -2734,12 +2739,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, bool is_aset)
{
PGresult *res;
PGresult *next_res;
@@ -2759,7 +2764,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 && !is_aset)
{
fprintf(stderr,
"client %d script %d command %d query %d: expected one row, got %d\n",
@@ -2769,17 +2774,24 @@ readCommandResponse(CState *st, char *varprefix)
break;
case PGRES_TUPLES_OK:
- if (is_last && varprefix != NULL)
+ if (varprefix != NULL && (is_last || is_aset))
{
- if (PQntuples(res) != 1)
+ int ntuples = PQntuples(res);
+
+ if (!is_aset && ntuples != 1)
{
+ /* under \gset, report the error */
fprintf(stderr,
"client %d script %d command %d query %d: expected one row, got %d\n",
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)
+ break;
+
+ /* store results into possibly prefixed variables */
for (int fld = 0; fld < PQnfields(res); fld++)
{
char *varname = PQfname(res, fld);
@@ -2788,15 +2800,14 @@ 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, is_aset ? "aset" : "gset", varname,
+ PQgetvalue(res, ntuples - 1, fld)))
{
/* internal error */
fprintf(stderr,
"client %d script %d command %d query %d: error storing into variable %s\n",
- st->id, st->use_file, st->command, qrynum,
- varname);
+ st->id, st->use_file, st->command, qrynum, varname);
goto error;
}
@@ -2838,7 +2849,7 @@ error:
{
res = PQgetResult(st->con);
PQclear(res);
- } while (res);
+ } while (res != NULL);
return false;
}
@@ -3213,7 +3224,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]->aset))
st->state = CSTATE_END_COMMAND;
else
st->state = CSTATE_ABORTED;
@@ -4447,6 +4460,7 @@ create_sql_command(PQExpBuffer buf, const char *source)
my_command->argc = 0;
memset(my_command->argv, 0, sizeof(my_command->argv));
my_command->varprefix = NULL; /* allocated later, if needed */
+ my_command->aset = false;
my_command->expr = NULL;
initSimpleStats(&my_command->stats);
@@ -4675,7 +4689,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],
@@ -4820,10 +4834,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;
@@ -4845,6 +4859,7 @@ ParseScript(const char *script, const char *desc, int weight)
cmd->varprefix = pg_strdup("");
else
cmd->varprefix = pg_strdup(command->argv[1]);
+ cmd->aset = (command->meta == META_ASET);
/* 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 1845869016..fc286a1af0 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -548,7 +548,7 @@ pgbench(
}
});
-# working \gset
+# working \gset and \aset
pgbench(
'-t 1', 0,
[ qr{type: .*/001_pgbench_gset}, qr{processed: 1/1} ],
@@ -558,9 +558,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
@@ -581,6 +583,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)
}
});
On 11/29/19 4:34 AM, 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.
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.
Michael, do you agree with Fabien's comments?
Attached v4 removes an unwanted rebased comment duplication and does
minor changes while re-reading the code.
This patch no longer applies: http://cfbot.cputube.org/patch_27_2091.log
CF entry has been updated to Waiting on Author.
Regards,
--
-David
david@pgmasters.net
On Tue, Mar 24, 2020 at 11:04:45AM -0400, David Steele wrote:
On 11/29/19 4:34 AM, Fabien COELHO wrote:
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.Michael, do you agree with Fabien's comments?
Thanks for the reminder. I am following up with Fabien's comments.
--
Michael
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
Attachments:
pgbench-aset-5.patchtext/x-diff; charset=us-asciiDownload
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>
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.
Attachments:
pgbench-aset-5.patchtext/x-diff; name=pgbench-aset-5.patchDownload
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)
}
});
On Thu, Mar 26, 2020 at 10:35:03PM +0100, Fabien COELHO wrote:
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.
Yes, that's the point I was trying to make. Thanks for sending a new
version.
- * 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.
I did not quite get why you need to update this comment. The same
concepts as before apply.
+ /* 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.).
+ /* coldly skip empty result under \aset */
+ else if (meta == META_ASET && ntuples <= 0)
+ break;
Yes, that's what I meant. Now it happens that we don't have a
regression test to cover the path where we have no tuples. Could it
be possible to add one?+ Assert((meta == META_NONE && varprefix == NULL) ||
+ ((meta == META_GSET || meta == META_ASET) && varprefix != NULL));
+
Good addition. That would blow up if meta is set to something else
than {META_NONE,META_GSET,META_ASET}, so anybody changing this code
path will need to question if he/she needs to do something here.Except for the addition of a test case to skip empty results when
\aset is used, I think that we are pretty good here.
--
Michael
On Mon, Mar 30, 2020 at 03:30:58PM +0900, Michael Paquier wrote:
Except for the addition of a test case to skip empty results when
\aset is used, I think that we are pretty good here.
While hacking on the patch more by myself, I found that mixing tests
for \gset and \aset was rather messy. A test for an empty result
leads also to a failure with the pgbench command as we want to make
sure that the variable does not exist in this case using debug(). So
let's split the tests in three parts:
- the set for \get is left alone.
- addition of a new set for the valid cases of \aset.
- addition of an invalid test for \aset (the empty set one).
Fabien, what do you think about the attached? Perhaps we should also
have a test where we return more than 1 row for \get? The last point
is unrelated to this thread though.
--
Michael
Attachments:
pgbench-aset-6.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index b8864c6ae5..354231e82d 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,9 +510,10 @@ 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.
+ * aset do gset on all possible queries of a combined query (\;).
* expr Parsed expression, if needed.
* stats Time spent in this command.
*/
@@ -2489,6 +2491,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 +2715,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 +2743,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 +2752,24 @@ 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;
}
+ else if (meta == META_ASET && ntuples <= 0)
+ {
+ /* coldly skip empty result under \aset */
+ 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..82a115c6d7 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -626,6 +626,42 @@ SELECT 0 AS i4, 4 AS i4 \gset
\set i debug(:i5)
}
});
+# working \aset
+# Valid cases.
+pgbench(
+ '-t 1', 0,
+ [ qr{type: .*/001_pgbench_aset}, qr{processed: 1/1} ],
+ [ qr{command=3.: int 8\b}, qr{command=4.: int 7\b} ],
+ 'pgbench aset command',
+ {
+ '001_pgbench_aset' => q{
+-- 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)
+}
+ });
+# Empty result set, causing command to fail.
+$node->safe_psql('postgres', 'CREATE UNLOGGED TABLE empty_tab (a int);');
+pgbench(
+ '-t 1', 2,
+ [ qr{type: .*/001_pgbench_aset_empty}, qr{processed: 0/1} ],
+ [
+ qr{undefined variable \"i8\"},
+ qr{evaluation of meta-command failed\b}
+ ],
+ 'pgbench aset command with empty result',
+ {
+ '001_pgbench_aset_empty' => q{
+-- empty result
+\; SELECT a as i8 FROM empty_tab\; \aset
+\set i debug(:i8)
+ }
+ });
+
+$node->safe_psql('postgres', 'DROP TABLE empty_tab;');
# trigger many expression errors
my @errors = (
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>
Bonjour Michaël,
Except for the addition of a test case to skip empty results when
\aset is used, I think that we are pretty good here.While hacking on the patch more by myself, I found that mixing tests
for \gset and \aset was rather messy. A test for an empty result
leads also to a failure with the pgbench command as we want to make
sure that the variable does not exist in this case using debug().
ISTM that I submitted a patch to test whether a variable exists in
pgbench, like available in psql (:{?var} I think), but AFAICS it did not
pass. Maybe I should resurect it as it would allow to test simply whether
an empty result was returned to aset, which could make sense in a bench
script (get something, if it does not exist skip remainder… I can see some
interesting use cases).
So let's split the tests in three parts:
- the set for \get is left alone.
- addition of a new set for the valid cases of \aset.
- addition of an invalid test for \aset (the empty set one).
Ok.
Fabien, what do you think about the attached?
It does not need to create an UNLOGGED table, a mere "WHERE FALSE"
suffices.
I do not understand why you removed the comment about meta which makes it
false, so I added something minimal back.
Perhaps we should also have a test where we return more than 1 row for
\get? The last point is unrelated to this thread though.
Yes, but ISTM that it is not worth a dedicated patch… so I added a test
similar to the one about empty aset.
See v7 attached.
--
Fabien.
Attachments:
pgbench-aset-7.patchtext/x-diff; name=pgbench-aset-7.patchDownload
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..798a952a25 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,15 @@ 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. On SQL_COMMAND: META_NONE/GSET/ASET.
* 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 +2491,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 +2715,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 +2743,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 +2752,24 @@ 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;
}
+ else if (meta == META_ASET && ntuples <= 0)
+ {
+ /* coldly skip empty result under \aset */
+ 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..cfac3fe138 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -624,6 +624,53 @@ 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)
+}
+ });
+# More that one row for gset, causing command to fail.
+pgbench(
+ '-t 1', 2,
+ [ qr{type: .*/001_pgbench_gset_two_rows}, qr{processed: 0/1} ],
+ [
+ qr{expected one row, got 2\b}
+ ],
+ 'pgbench gset command with two rows',
+ {
+ '001_pgbench_gset_two_rows' => q{
+SELECT 5432 AS fail UNION SELECT 5433 ORDER BY 1 \gset
+}
+ });
+
+# working \aset
+# Valid cases.
+pgbench(
+ '-t 1', 0,
+ [ qr{type: .*/001_pgbench_aset}, qr{processed: 1/1} ],
+ [ qr{command=3.: int 8\b}, qr{command=4.: int 7\b} ],
+ 'pgbench aset command',
+ {
+ '001_pgbench_aset' => q{
+-- 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)
+}
+ });
+# Empty result set, causing command to fail.
+pgbench(
+ '-t 1', 2,
+ [ qr{type: .*/001_pgbench_aset_empty}, qr{processed: 0/1} ],
+ [
+ qr{undefined variable \"i8\"},
+ qr{evaluation of meta-command failed\b}
+ ],
+ 'pgbench aset command with empty result',
+ {
+ '001_pgbench_aset_empty' => q{
+-- empty result
+\; SELECT 5432 AS i8 WHERE FALSE \; \aset
+\set i debug(:i8)
}
});
On Thu, Apr 02, 2020 at 08:08:08AM +0200, Fabien COELHO wrote:
ISTM that I submitted a patch to test whether a variable exists in pgbench,
like available in psql (:{?var} I think), but AFAICS it did not pass. Maybe
I should resurect it as it would allow to test simply whether an empty
result was returned to aset, which could make sense in a bench script (get
something, if it does not exist skip remainder… I can see some interesting
use cases).
Not sure if improving the readability of the tests is a reason for
this patch. So I would suggest to just live with relying on debug()
for now to check that a variable with a given prefix exists.
It does not need to create an UNLOGGED table, a mere "WHERE FALSE" suffices.
Good point, that's cheaper.
I do not understand why you removed the comment about meta which makes it
false, so I added something minimal back.
* 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. On SQL_COMMAND: META_NONE/GSET/ASET.Oh, OK. I see your point. Sorry about that.
Perhaps we should also have a test where we return more than 1 row for
\get? The last point is unrelated to this thread though.Yes, but ISTM that it is not worth a dedicated patch… so I added a test
similar to the one about empty aset.
Thanks. So, it looks like everything has been addressed. Do you have
anything else in mind?
NB: I think that it is really strange to not use an array for the
options in subroutine pgbench() of 001_pgbench_with_server.pl.
Shouldn't this be an array of options instead? The current logic of
using a splitted string is weak when it comes to option quoting in
perl and command handling in general.
--
Michael
Michaï¿œl,
ISTM that I submitted a patch to test whether a variable exists in pgbench,
like available in psql (:{?var} I think),Not sure if improving the readability of the tests is a reason for
this patch. So I would suggest to just live with relying on debug()
for now to check that a variable with a given prefix exists.
Sure. I meant that the feature would make sense to write benchmark scripts
which would use aset and be able to act on the success or not of this
aset, not to resurrect it for a hidden coverage test.
Thanks. So, it looks like everything has been addressed. Do you have
anything else in mind?
Nope.
NB: I think that it is really strange to not use an array for the
options in subroutine pgbench() of 001_pgbench_with_server.pl.
Shouldn't this be an array of options instead? The current logic of
using a splitted string is weak when it comes to option quoting in
perl and command handling in general.
The idea is that a scalar is simpler and readable to write in the simple
case than a perl array. Now maybe qw() could have done the trick.
--
Fabien.
On Thu, Apr 02, 2020 at 03:58:50PM +0200, Fabien COELHO wrote:
Sure. I meant that the feature would make sense to write benchmark scripts
which would use aset and be able to act on the success or not of this aset,
not to resurrect it for a hidden coverage test.
This could always be discussed for v14. We'll see.
Thanks. So, it looks like everything has been addressed. Do you have
anything else in mind?Nope.
Applied, then. Thanks!
--
Michael
Bonjour Michaï¿œl,
Sure. I meant that the feature would make sense to write benchmark scripts
which would use aset and be able to act on the success or not of this aset,
not to resurrect it for a hidden coverage test.This could always be discussed for v14. We'll see.
Or v15, or never, who knows? :-)
The use case I have in mind for such a feature is to be able to have a
flow of DELETE transactions in a multi-script benchmark without breaking
concurrent SELECT/UPDATE transactions. For that, the ability of extracting
data easily and testing whether it was non empty would help.
Applied, then. Thanks!
Thanks to you!
--
Fabien.