Re: Improve error reporting in 027_stream_regress test
Nazir Bilal Yavuz <byavuz81@gmail.com>
From: Bilal Yavuz <byavuz81@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Brandon Tat <brandontat6@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Alexander Lakhin <exclusion@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
Andres Freund <andres@anarazel.de>
Date: 2025-12-04T12:33:30Z
Lists: pgsql-hackers
Attachments
- v5-0001-Add-read_file_ends-helper-to-Utils.pm.patch (text/x-patch)
- v5-0002-Improve-error-reporting-in-027_stream_regress-tes.patch (text/x-patch)
Hi Michael, Brandon and Ben, Thank you for looking into this! On Thu, 4 Dec 2025 at 10:42, Michael Paquier <michael@paquier.xyz> wrote: > > On Wed, Dec 03, 2025 at 11:01:31PM -0800, Brandon Tat wrote: > > Regarding the function regression_log_helper(), this function reads > > all the lines in the logs at line 219 of > > src/test/recovery/t/027_stream_regress.pl. It seems wasteful to read > > the file again twice in read_file_ends(). Alternatively, we could > > read the file once within regression_log_helper() and index lines to > > emit the lines that we want. > > It seems to me that you are looking at v4-0001 and v4-0002 posted at > [1], which I did not author. So your suggestion would be to call > read_file_ends() once with the file opened once, with three modes > instead of the two presented in the patch: fetch the head, the tail, > or both at the same time. Yes, that would be more efficient. > > While looking at the patch with fresher eyes (didn't look at this > thread for a couple of months, sorry), it looks like there is no point > in having regression_log_helper() at all. We could just return the > tail and the head in a single call of read_file_ends() with two output > variables. Then we could embed in read_file_ends() the knowledge that > if we are dealing with a file that has less lines than twice > PG_TEST_FILE_READ_LINES, we can just print the whole file, returning > only the full contents as in a variable for what would have been the > head content, leaving the tail content empty. > > If somebody would like to send a patch among these lines, feel free.. I applied these feedbacks in v5. I wanted to cover all possible cases so I think 0002 might be a bit more complicated than it needs to be. What do you think about the current implementation? -- Regards, Nazir Bilal Yavuz Microsoft