Horton

Re: Fix for Extra Parenthesis in pgbench progress message

Fujii Masao <masao.fujii@oss.nttdata.com>

View thread View on postgr.es
From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Tatsuo Ishii <ishii@postgresql.org>
Cc: btogiwarayuushi@oss.nttdata.com, pgsql-hackers@postgresql.org
Date: 2024-11-07T15:54:58Z
Lists: pgsql-hackers

Attachments


On 2024/11/07 10:41, Tatsuo Ishii wrote:
>>> The patch works perfectly for the case that there is one extra brace
>>> as shown in your example. However I think it will not work if there
>>> are two or more extra braces.
>>
>> Are you suggesting adding more space characters before the carriage
>> return
>> in the progress reporting line, like this?
> 
> No. I thought about dynamically calculating spaces needed to be
> printed something like attached patch.
> 
>> Since the line includes
>> both
>> elapsed and remaining times, its total length doesn’t change much.
> 
> Maybe. But I am also worried about the case when we would want to
> change the log line format in the future. We might introduce this kind
> of bug again.  By dynamically calculating the number of necessary
> spaces, we don't need to think about the concern.

+1

+			if (previous_chars != 0)
+			{
+				int	n_spaces = chars - previous_chars;
+				fprintf(stderr, "%*s", n_spaces, "");
+				chars += n_spaces;
+			}

Currently, this is added only when use_quiet is false, but shouldn’t it also apply when use_quiet is true?

Also, what happens if chars is smaller than previous_chars?

I’m unsure why chars needs to be incremented by n_spaces.


I’ve created an updated patch based on your idea—could you take a look?

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Commits

  1. Fix pgbench performance issue induced by commit af35fe501.

  2. pgbench: Ensure previous progress message is fully cleared when updating.