Re: pgbench tap tests & minor fixes

From: Nikolay Shaplov <dhyan(at)nataraj(dot)su>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: pgbench tap tests & minor fixes
Date: 2017-06-14 20:05:37
Message-ID: 1582822.gOaBPI6nxI@x200m
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

В письме от 8 июня 2017 19:56:02 пользователь Fabien COELHO написал:

> > So this should be fixed in both expr_scanner_get_substring cases, and keep
> > last symbol only if it is not "\n".
>
> Indeed, this is a mess. The code assumes all stuff is a line ending with
> '\n', but this is not always the case.
>
> Here is a v7 which chomps the final newline only if there is one.

Sorry, but I still have problems with new solution...

First is function name. "expr_scanner_get_line" does not deal with any line at
all... it gets substring and "chomps" it.

As for me, I would do expr_scanner_chomp_substring(PsqlScanState, int, int&);
that changes end_offset as desired... And use it instead of end_offset =
expr_scanner_offset(sstate) - 1;

The second issue: you are removing all trailing \n and \r. I think you should
remove only one \n at the end of the string, and one \r before \n if there was
one.
I do not think there will be any practical difference between my and yours
solutions here, but mine is more neat, I think... I do not have enough
imagination to think about if "\n\r\r\r\r\r\n" case can happen, and what will
happen of we remove them all... So I suggest to remove only the last one.

--
Do code for fun. Can do it for money (Perl & C/C++ ~10h/week)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-06-14 20:24:12 Re: RTE_NAMEDTUPLESTORE, enrtuples and comments
Previous Message Robert Haas 2017-06-14 20:03:23 Re: intermittent failures in Cygwin from select_parallel tests