Re: Fix for Extra Parenthesis in pgbench progress message - Mailing list pgsql-hackers

From Tatsuo Ishii
Subject Re: Fix for Extra Parenthesis in pgbench progress message
Date
Msg-id 20241108.114713.484828673471024549.ishii@postgresql.org
Whole thread Raw
List pgsql-hackers
>> 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?

Yes. The patch just showed my idea and I thought I needed to apply the
case when use_quiet is true.  But the fix should be applied to after
"else if (use_quiet && (j % 100 == 0))" as shown in your patch.

> Also, what happens if chars is smaller than previous_chars?

My oversight. Better to check if chars is smaller than previous_chars.

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

Yeah, it's not necessary.

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

Sure.

I think you need to adjust

        fprintf(stderr, "%*c\r", chars - 1, ' ');    /* Clear the current line */

to:

        fprintf(stderr, "%*c\r", chars, ' ');    /* Clear the current line */

since now chars does not consider the EOL char. By clearing chars - 1,
the closing parenthesis will be left on the screen.

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



pgsql-hackers by date:

Previous
From: Richard Guo
Date:
Subject: Re: Inconsistent RestrictInfo serial numbers
Next
From: Amit Kapila
Date:
Subject: Re: [bug fix] prepared transaction might be lost when max_prepared_transactions is zero on the subscriber