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 |
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 |