pgbench failed when -f option contains a char '@'
Hi, hackers
pgbench use -f filename[@weight] to receive a sql script file with a weight,
but if I create a file contains char'@', like a@2.sql, specify this file without weigth,
pgbench will failed with error:
pgbench: fatal: invalid weight specification: @2.sql
This action may be unfriendly, because the char '@' is a valid character on Linux
and Windows.
I have created a patch to modify this action. The patch is attached.
Thoughts?
Regards
Shenhao Wang
Attachments:
0001-Improve-pgbench-when-f-option-contain-char-in-filepa.patchapplication/octet-stream; name=0001-Improve-pgbench-when-f-option-contain-char-in-filepa.patchDownload
From 7ad5a2aabab2393406f632e2559474af8af1382c Mon Sep 17 00:00:00 2001
From: Shenhao Wang <wangsh.fnst@cn.fujitsu.com>
Date: Fri, 18 Dec 2020 11:36:08 +0800
Subject: [PATCH] Improve pgbench when -f option contain char '@' in filepath
---
src/bin/pgbench/pgbench.c | 43 +++++++++++++++-----
src/bin/pgbench/t/001_pgbench_with_server.pl | 4 +-
2 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 3057665bbe..ceebb38409 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -4925,24 +4925,37 @@ read_file_contents(FILE *fd)
}
/*
- * Given a file name, read it and add its script to the list.
+ * Given a file name, and open it
* "-" means to read stdin.
* NB: filename must be storage that won't disappear.
*/
-static void
-process_file(const char *filename, int weight)
+static FILE*
+open_file(const char * filename, bool error_exit)
{
FILE *fd;
- char *buf;
-
- /* Slurp the file contents into "buf" */
if (strcmp(filename, "-") == 0)
fd = stdin;
- else if ((fd = fopen(filename, "r")) == NULL)
+ else if ((fd = fopen(filename, "r")) == NULL && error_exit)
{
pg_log_fatal("could not open file \"%s\": %m", filename);
exit(1);
}
+ return fd;
+}
+
+/*
+ * Given a FILE ptr, read it and add its script to the list.
+ * if FILE ptr is null, open file with filename
+ * NB: filename must be storage that won't disappear.
+ */
+static void
+process_file(FILE *fd, int weight, const char *filename)
+{
+ char *buf;
+
+ /* Slurp the file contents into "buf" */
+ if (fd == NULL)
+ fd = open_file(filename, true);
buf = read_file_contents(fd);
@@ -5633,10 +5646,18 @@ main(int argc, char **argv)
internal_script_used = true;
break;
case 'f':
- weight = parseScriptWeight(optarg, &script);
- process_file(script, weight);
- benchmarking_option_set = true;
- break;
+ {
+ FILE *fd = open_file(optarg, false);
+ if (fd != NULL)
+ process_file(fd, 1, optarg);
+ else
+ {
+ weight = parseScriptWeight(optarg, &script);
+ process_file(NULL, weight, script);
+ }
+ benchmarking_option_set = true;
+ break;
+ }
case 'D':
{
char *p;
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 61b671d54f..ecb8024ef0 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -233,14 +233,14 @@ pgbench(
'-n -t 10 -c 1 -M simple',
0,
[
- qr{type: .*/001_pgbench_custom_script_3},
+ qr{type: .*/001_pgbench_\@custom_script_3},
qr{processed: 10/10},
qr{mode: simple}
],
[qr{^$}],
'pgbench custom script',
{
- '001_pgbench_custom_script_3' => q{-- select only variant
+ '001_pgbench_@custom_script_3' => q{-- select only variant
\set aid random(1, :scale * 100000)
BEGIN;
SELECT abalance::INTEGER AS balance
--
2.26.2
On 18/12/2020 08:22, Wang, Shenhao wrote:
Hi, hackers
pgbench use -f filename[@weight] to receive a sql script file with a weight,
but if I create a file contains char'@', like a@2.sql, specify this file without weigth,
pgbench will failed with error:
pgbench: fatal: invalid weight specification: @2.sqlThis action may be unfriendly, because the char '@' is a valid character on Linux
and Windows.I have created a patch to modify this action. The patch is attached.
This patch changes it to first check if the file "a@2.sql" exists, and
if it doesn't, only then it tries to interpret it as a weight, as
filename "a" and weight "2.sql". That stilll doesn't fix the underlying
ambiguity, though. If you have a file called "script" and "script@1",
this makes it impossible to specify "script" with weight 1, because "-f
script@1" will now always open the file "script@1".
I think we should just leave this as it is. The user can simply rename
the file.
Or maybe one change would be worthwhile here: First check if the part
after the @ contains only digits. If doesn't, then assume it's part of
the filename rather than a weight. That would fix this for cases like
"foo@1.sql", although not for "foo@1".
- Heikki
Heikki Linnakangas <hlinnaka@iki.fi> writes:
I think we should just leave this as it is. The user can simply rename
the file.
Yeah. The assumption when we defined the script-weight syntax was that
there's no particular reason to use "@" in a script file name, and
I don't see why that's a bad assumption.
Or maybe one change would be worthwhile here: First check if the part
after the @ contains only digits. If doesn't, then assume it's part of
the filename rather than a weight. That would fix this for cases like
"foo@1.sql", although not for "foo@1".
I do not like introducing ambiguity of that sort. Not being entirely
clear on which script file is going to be read seems like a recipe
for security issues.
regards, tom lane
Hello,
pgbench use -f filename[@weight] to receive a sql script file with a weight,
ISTM that I thought of this: "pgbench -f filen@me@1" does work.
sh> touch foo@bla
sh> pgbench -f foo@bla@1
pgbench: fatal: empty command list for script "foo@bla"
The documentation could point this out, though.
--
Fabien.
Hello Tom,
I think we should just leave this as it is. The user can simply rename
the file.Yeah. The assumption when we defined the script-weight syntax was that
there's no particular reason to use "@" in a script file name, and
I don't see why that's a bad assumption.
The "parser" looks for the last @ in the argument, so the simple
workaround is to append "@1".
I suggest the attached doc update, or anything in better English.
--
Fabien.
Attachments:
pgbench-weight-doc-1.patchtext/x-diff; name=pgbench-weight-doc-1.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 7180fedd65..bba3cf05b0 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -461,6 +461,8 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
the list of executed scripts.
An optional integer weight after <literal>@</literal> allows to adjust the
probability of drawing the test.
+ If the filename includes a <literal>@</literal> character, append a weight so
+ that there is no ambiguity: <literal>filen@me@1</literal>.
See below for details.
</para>
</listitem>