Escape output of pg_amcheck test

Started by Peter Eisentrautabout 2 years ago7 messages
#1Peter Eisentraut
peter@eisentraut.org
1 attachment(s)

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.

Attachments:

0001-Escape-output-of-pg_amcheck-test.patchtext/plain; charset=UTF-8; name=0001-Escape-output-of-pg_amcheck-test.patchDownload
From 09d82d19e045114b1e2e7c3751e96cbae1eaaa0c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 8 Jan 2024 08:23:26 +0100
Subject: [PATCH] Escape output of pg_amcheck test

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.
---
 src/bin/pg_amcheck/t/004_verify_heapam.pl | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/bin/pg_amcheck/t/004_verify_heapam.pl b/src/bin/pg_amcheck/t/004_verify_heapam.pl
index 8c796ed221..f6d2c5f787 100644
--- a/src/bin/pg_amcheck/t/004_verify_heapam.pl
+++ b/src/bin/pg_amcheck/t/004_verify_heapam.pl
@@ -367,10 +367,11 @@ sub write_tuple
 	{
 		close($file);    # ignore errors on close; we're exiting anyway
 		$node->clean_node;
-		plan skip_all =>
-		  sprintf(
+		plan skip_all => sprintf(
 			"Page layout of index %d differs from our expectations: expected (%x, %x, \"%s\"), got (%x, %x, \"%s\")",
-			$tupidx, 0xDEADF9F9, 0xDEADF9F9, "abcdefg", $a_1, $a_2, $b);
+			$tupidx, 0xDEADF9F9, 0xDEADF9F9, "abcdefg", $a_1, $a_2,
+			# escape non-word characters to avoid confusing the terminal
+			$b =~ s{(\W)}{ sprintf '\x%02x', ord($1) }aegr);
 		exit;
 	}
 
-- 
2.43.0

#2Aleksander Alekseev
aleksander@timescale.com
In reply to: Peter Eisentraut (#1)
Re: Escape output of pg_amcheck test

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]https://www.postgresql.org/docs/current/install-requirements.html.

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

--
Best regards,
Aleksander Alekseev

#3Mark Dilger
hornschnorter@gmail.com
In reply to: Peter Eisentraut (#1)
Re: Escape output of pg_amcheck test

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

#4Mark Dilger
mark.dilger@enterprisedb.com
In reply to: Mark Dilger (#3)
Re: Escape output of pg_amcheck test

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

#5Aleksander Alekseev
aleksander@timescale.com
In reply to: Mark Dilger (#4)
Re: Escape output of pg_amcheck test

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

#6Peter Eisentraut
peter@eisentraut.org
In reply to: Aleksander Alekseev (#5)
Re: Escape output of pg_amcheck test

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. :)

#7Peter Eisentraut
peter@eisentraut.org
In reply to: Peter Eisentraut (#6)
Re: Escape output of pg_amcheck test

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