Re: pgbench - refactor init functions with buffers

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Jeevan Ladhe <jeevan(dot)ladhe(at)enterprisedb(dot)com>
Cc: Dilip Kumar <dilipbalaut(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgbench - refactor init functions with buffers
Date: 2019-10-22 11:06:20
Message-ID: alpine.DEB.2.21.1910221243311.15559@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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.

Attachment Content-Type Size
pgbench-buffer-2.patch text/x-diff 11.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2019-10-22 11:20:36 Re: dropdb --force
Previous Message Amit Kapila 2019-10-22 10:43:03 Re: Proposal: Make use of C99 designated initialisers for nulls/values arrays