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-07 16:59:22
Message-ID: 2161488.LtOUyK07j0@x200m
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

В письме от 30 мая 2017 17:24:26 Вы написали:

> > I still have three more questions. A new one:
> >
> > ========
> >
> > my_command->line = expr_scanner_get_substring(sstate,
> >
> > start_offset,
> >
> > - end_offset);
> > + end_offset + 1);
> > ========
> >
> > I do not quite understand what are you fixing here,
>
> I fix a truncation which appears in an error message later on.
>
> > I did not find any mention of it in the patch introduction letter.
>
> Indeed. Just a minor bug fix to avoid an error message to be truncated. If
> you remove it, then you can get:
>
> missing argument in command "sleep"
> \slee
>
> Note the missing "p"...
>
> > And more, expr_scanner_get_substring is called twice in very similar
> > code, and it is fixed only once. Can you tell more about this fix.
>
> I fixed the one which was generating truncated messages. I did not notice
> other truncated messages while testing, so I assume that other calls are
> correct, but I did not investigate further, so I may be wrong. Maybe in
> other instances the truncation removes a "\n" which is intended?

I did some investigation: The code there really suppose that there is always
\n at the end of the line, and truncates the line. It is done in

/* Get location of the ending newline */
end_offset = expr_scanner_offset(sstate) - 1;

just two lines above the code we are discussing.

When you have one line code /sleep 2ms with no "end of line" symbol at the
end, it will cut off "s" instead of "\n"

You've fix it, but now it will leave \n, in all sleeps in multiline scripts.

So this should be fixed in both expr_scanner_get_substring cases, and keep last
symbol only if it is not "\n".

> Here is a v6.
>
> - it uses "progress == 0"
>
> - it does not change "nxacts <= 0" because of possible wrapping
>
> - it fixes two typos in a perl comment about the checks_all function
>
> - I kept "checks all" because this talks more to me than "like more"
> if a native English speaker or someone else has an opinion, it is
> welcome.
Ok, let's leave this for commiter to make final decision.

> Also, if someone could run a test on windows, it would be great.
I'll try to ask a friend of mine to run this on windows...

--
Nikolay Shaplov, independent Perl & C/C++ developer. Available for hire.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-06-07 17:07:10 Re: BEFORE trigger can cause undetected partition constraint violation
Previous Message Andres Freund 2017-06-07 16:49:23 Re: Challenges preventing us moving to 64 bit transaction id (XID)?