Re: pgbench - refactor init functions with buffers

Fabien COELHO <coelho@cri.ensmp.fr>

From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2019-10-22T11:06:20Z
Lists: pgsql-hackers

Attachments

Hello Jeevan,

>> I haven't read the complete patch.  But, I have noticed that many
>> places you changed the variable declaration from c to c++ style (i.e
>> moved the declaration in the for loop).  IMHO, generally in PG, we
>> don't follow this convention.  Is there any specific reason to do
>> this?
>
> +1.

As I said, this C99 feature is already used extensively in pg sources, so 
it makes sense to use it when refactoring something and if appropriate, 
which IMO is the case here.

> The patch does not apply on master, needs rebase.

Hmmm. "git apply pgbench-buffer-1.patch" works for me on current master.

> Also, I got some whitespace errors.

It possible, but I cannot see any. Could you be more specific?

Many mailers do not conform to MIME and mess-up newlines when attachements 
are typed text/*, because MIME requires the mailer to convert those to 
crnl eol when sending and back to system eol when receiving, but few 
actually do it. Maybe the issue is really there.

> I think you can also refactor the function tryExecuteStatement(), and
> call your newly added function executeStatementExpect() by passing
> an additional flag something like "errorOK".

Indeed, good point.

-- 
Fabien.

Commits

  1. pgbench: Use PQExpBuffer to simplify code that constructs SQL.

  2. Make command order in test more sensible