Horton

Thread

Commits

  1. Escape output of pg_amcheck test

  1. Escape output of pg_amcheck test

    Peter Eisentraut <peter@eisentraut.org> — 2024-01-08T07:27:57Z

    The pg_amcheck reports a skip message if the layout of the index does 
not match expectations.  That message includes the bytes that were 
expected and the ones that were found.  But the found ones are arbitrary 
bytes, which can have funny effects on the terminal when they are 
printed.  To avoid that, escape non-word characters before printing.

  2. Re: Escape output of pg_amcheck test

    Aleksander Alekseev <aleksander@timescale.com> — 2024-01-08T12:45:06Z

    Hi,

> The pg_amcheck reports a skip message if the layout of the index does
> not match expectations.  That message includes the bytes that were
> expected and the ones that were found.  But the found ones are arbitrary
> bytes, which can have funny effects on the terminal when they are
> printed.  To avoid that, escape non-word characters before printing.

LGTM.

I didn't get the part about the /r modifier at first, but "man perlre" helped:

"""
r  - perform non-destructive substitution and return the new value
"""

The /a modifier requires Perl >= 5.14, which is fine [1].

[1]: https://www.postgresql.org/docs/current/install-requirements.html

-- 
Best regards,
Aleksander Alekseev

  3. Re: Escape output of pg_amcheck test

    Mark Dilger <hornschnorter@gmail.com> — 2024-01-08T13:41:02Z

    
On 1/7/24 23:27, Peter Eisentraut wrote:
> The pg_amcheck reports a skip message if the layout of the index does 
> not match expectations.  That message includes the bytes that were 
> expected and the ones that were found.  But the found ones are arbitrary 
> bytes, which can have funny effects on the terminal when they are 
> printed.  To avoid that, escape non-word characters before printing.

> +			# escape non-word characters to avoid confusing the terminal
> +			$b =~ s{(\W)}{ sprintf '\x%02x', ord($1) }aegr);

The /r modifier defeats the purpose of the patch, at least for my perl 
version, perl 5, version 28, subversion 1 (v5.28.1).  With just the /aeg 
modifier, it works fine.

-- 
Mark Dilger

  4. Re: Escape output of pg_amcheck test

    Mark Dilger <mark.dilger@enterprisedb.com> — 2024-01-08T13:52:26Z

    
> On Jan 8, 2024, at 5:41 AM, Mark Dilger <hornschnorter@gmail.com> wrote:
> 
> The /r modifier defeats the purpose of the patch, at least for my perl version, perl 5, version 28, subversion 1 (v5.28.1).  With just the /aeg modifier, it works fine.

Nevermind.  I might be wrong about that.  I didn't have a test case handy that would generate index corruption which would result in characters of the problematic class, and so I quickly wrote some (wrong) instrumentation to try to test your patch.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  5. Re: Escape output of pg_amcheck test

    Aleksander Alekseev <aleksander@timescale.com> — 2024-01-08T14:04:19Z

    Hi,

> [...] so I quickly wrote some (wrong) instrumentation to try to test your patch.

Yep, it confused me too at first.

Since the encoding happens right before exit() call, maybe it's worth
changing $b in-place in order to make the code slightly more readable
for most of us :)

-- 
Best regards,
Aleksander Alekseev

  6. Re: Escape output of pg_amcheck test

    Peter Eisentraut <peter@eisentraut.org> — 2024-01-08T15:06:01Z

    On 08.01.24 15:04, Aleksander Alekseev wrote:
>> [...] so I quickly wrote some (wrong) instrumentation to try to test your patch.
> 
> Yep, it confused me too at first.
> 
> Since the encoding happens right before exit() call, maybe it's worth
> changing $b in-place in order to make the code slightly more readable
> for most of us :)

My patch originally had the old-style

my $b_escaped = $b;
$b_escaped =~ s/.../;

... sprintf(..., $b_escaped);

but then I learned about the newish /r modifier and thought it was 
cooler. :)

  7. Re: Escape output of pg_amcheck test

    Peter Eisentraut <peter@eisentraut.org> — 2024-01-14T06:32:05Z

    On 08.01.24 16:06, Peter Eisentraut wrote:
> On 08.01.24 15:04, Aleksander Alekseev wrote:
>>> [...] so I quickly wrote some (wrong) instrumentation to try to test 
>>> your patch.
>>
>> Yep, it confused me too at first.
>>
>> Since the encoding happens right before exit() call, maybe it's worth
>> changing $b in-place in order to make the code slightly more readable
>> for most of us :)
> 
> My patch originally had the old-style
> 
> my $b_escaped = $b;
> $b_escaped =~ s/.../;
> 
> ... sprintf(..., $b_escaped);
> 
> but then I learned about the newish /r modifier and thought it was 
> cooler. :)

committed