Typo in pgbench messages.

Hi,

I found messages inserted a space before the "%" in pgbench.
I think this is typo because there are no space before the "%" in other messages.
What do you think?

I think you are right. In English there's should be no space between number and "%".
AFAIK other parts of PostgreSQL follow the rule.

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index f166a77e3a..4ebe5e6ea4 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5598,11 +5598,11 @@ printResults(StatsData *total,
return;

if (throttle_delay && latency_limit)
-		printf("number of transactions skipped: " INT64_FORMAT " (%.3f %%)\n",
+		printf("number of transactions skipped: " INT64_FORMAT " (%.3f%%)\n",
total->skipped, 100.0 * total->skipped / total->cnt);

if (latency_limit)
-		printf("number of transactions above the %.1f ms latency limit: " INT64_FORMAT "/" INT64_FORMAT " (%.3f %%)\n",
+		printf("number of transactions above the %.1f ms latency limit: " INT64_FORMAT "/" INT64_FORMAT " (%.3f%%)\n",
latency_limit / 1000.0, latency_late, ntx,
(ntx > 0) ? 100.0 * latency_late / ntx : 0.0);

Looks good to me.

Best reagards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

Thanks for your comment!

Sorry, I did not attach the patch file.
This patch focas on master branch.

Best regards,

On Thu, 24 Feb 2022 12:15:55 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

Hi,

I found messages inserted a space before the "%" in pgbench.
I think this is typo because there are no space before the "%" in other messages.
What do you think?

I think you are right. In English there's should be no space between number and "%".
AFAIK other parts of PostgreSQL follow the rule.

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index f166a77e3a..4ebe5e6ea4 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5598,11 +5598,11 @@ printResults(StatsData *total,
return;

if (throttle_delay && latency_limit)
-		printf("number of transactions skipped: " INT64_FORMAT " (%.3f %%)\n",
+		printf("number of transactions skipped: " INT64_FORMAT " (%.3f%%)\n",
total->skipped, 100.0 * total->skipped / total->cnt);

if (latency_limit)
-		printf("number of transactions above the %.1f ms latency limit: " INT64_FORMAT "/" INT64_FORMAT " (%.3f %%)\n",
+		printf("number of transactions above the %.1f ms latency limit: " INT64_FORMAT "/" INT64_FORMAT " (%.3f%%)\n",
latency_limit / 1000.0, latency_late, ntx,
(ntx > 0) ? 100.0 * latency_late / ntx : 0.0);

Looks good to me.

Best reagards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

--
KAWAMOTO Masaya <kawamoto@sraoss.co.jp>
SRA OSS, Inc. Japan

I think you are right. In English there's should be no space between number and "%".
AFAIK other parts of PostgreSQL follow the rule.

I think it's better to back-patch this to stable branches if there's
no objection. Thought?

Best reagards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

On Thu, Feb 24, 2022 at 06:38:57PM +0900, Tatsuo Ishii wrote:

I think you are right. In English there's should be no space between number and "%".
AFAIK other parts of PostgreSQL follow the rule.

I think it's better to back-patch this to stable branches if there's
no objection. Thought?

That's only cosmetic, so I would just bother about HEAD if I were to
change something like that (I would not bother at all, personally).

One argument against a backpatch is that this would be disruptive with
tools that parse and analyze the output generated by pgbench. Fabien,
don't you have some tools and/or wrappers doing exactly that?
--
Michael

Bonjour Michaï¿œl,

I think it's better to back-patch this to stable branches if there's
no objection. Thought?

That's only cosmetic, so I would just bother about HEAD if I were to
change something like that (I would not bother at all, personally).

One argument against a backpatch is that this would be disruptive with
tools that parse and analyze the output generated by pgbench. Fabien,
don't you have some tools and/or wrappers doing exactly that?

Yep, I like to "| cut -d' ' -fx" and other "line.split(' ')" or whatever.

I think that the break of typographical rules is intentional to allow such
simplistic low-level stream handling through pipes or scripts. I'd prefer
that the format is not changed. Maybe a comment could be added to explain
the reason behind it.

--
Fabien.

On 24 Feb 2022, at 13:58, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

One argument against a backpatch is that this would be disruptive with
tools that parse and analyze the output generated by pgbench. Fabien,
don't you have some tools and/or wrappers doing exactly that?

Yep, I like to "| cut -d' ' -fx" and other "line.split(' ')" or whatever.

I think that the break of typographical rules is intentional to allow such simplistic low-level stream handling through pipes or scripts. I'd prefer that the format is not changed. Maybe a comment could be added to explain the reason behind it.

That doesn't sound like an overwhelmingly convincing argument to print some
messages with 'X %' and others with 'X%'.

--
Daniel Gustafsson https://vmware.com/

On Thu, 24 Feb 2022 14:44:05 +0100
Daniel Gustafsson <daniel@yesql.se> wrote:

On 24 Feb 2022, at 13:58, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

One argument against a backpatch is that this would be disruptive with
tools that parse and analyze the output generated by pgbench. Fabien,
don't you have some tools and/or wrappers doing exactly that?

Yep, I like to "| cut -d' ' -fx" and other "line.split(' ')" or whatever.

I think that the break of typographical rules is intentional to allow such simplistic low-level stream handling through pipes or scripts. I'd prefer that the format is not changed. Maybe a comment could be added to explain the reason behind it.

That doesn't sound like an overwhelmingly convincing argument to print some
messages with 'X %' and others with 'X%'.

I agree with Daniel. If inserting a space in front of % was intentional for
handling the result in such tools, we should fix other places where 'X%' is
used in the pgbench output.

Regards,
Yugo Nagata

--
Yugo NAGATA <nagata@sraoss.co.jp>

Hello Daniel,

I think that the break of typographical rules is intentional to allow
such simplistic low-level stream handling through pipes or scripts. I'd
prefer that the format is not changed. Maybe a comment could be added
to explain the reason behind it.

That doesn't sound like an overwhelmingly convincing argument to print
some messages with 'X %' and others with 'X%'.

Indeed. The no-space % are for database loading progress and the final
report which I happen not to process through pipes and are more
user-facing interfaces/reports. The stream piping need applies more to
repeated lines such as those output by progress, which happen to avoid
percentages anyway so the questions does not arise.

So fine with me wrt having a more homogeneous report.

--
Fabien.

Hi Fabien,

That doesn't sound like an overwhelmingly convincing argument to print
some messages with 'X %' and others with 'X%'.

Indeed. The no-space % are for database loading progress and the final
report which I happen not to process through pipes and are more
user-facing interfaces/reports. The stream piping need applies more to
repeated lines such as those output by progress, which happen to avoid
percentages anyway so the questions does not arise.

So fine with me wrt having a more homogeneous report.

So are you fine with Kawamoto-san's patch?

Best reagards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

So fine with me wrt having a more homogeneous report.

So are you fine with Kawamoto-san's patch?

Yes.

Patch applies cleanly (hmmm, it would have been better to have it as an
attachement). Make & make check ok. Fine with me.

--
Fabien.

So are you fine with Kawamoto-san's patch?

Yes.

Patch applies cleanly (hmmm, it would have been better to have it as
an attachement). Make & make check ok. Fine with me.

Thank you for the review. Fix pushed to master branch.

Best reagards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp