Thread

Re: Improve error reporting in 027_stream_regress test

Nazir Bilal Yavuz <byavuz81@gmail.com> — 2025-12-04T12:33:30Z

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