From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
Cc: | btendouan <btendouan(at)oss(dot)nttdata(dot)com>, "ibrar(dot)ahmad(at)gmail(dot)com:" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pgbench - extend initialization phase control |
Date: | 2019-11-05 21:22:51 |
Message-ID: | alpine.DEB.2.21.1911052208240.14337@lancre |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello,
>>> - for (step = initialize_steps; *step != '\0'; step++)
>>> + for (const char *step = initialize_steps; *step != '\0'; step++)
>
> But I still wonder why we should apply such change here.
Because it removes one declaration and reduces the scope of one variable?
> If there is the reason why this change is necessary here,
Nope, such changes are never necessary.
> I'm OK with that. But if not, basically I'd like to avoid the change.
> Otherwise it may make the back-patch a bit harder
> when we change the surrounding code.
I think that this is small enough so that it can be managed, if any back
patch occurs on the surrounding code, which is anyway pretty unlikely.
> Attached is the slightly updated version of the patch. Based on your
> patch, I added the descriptions about logging of "g" and "G" steps into
> the doc, and did some cosmetic changes. Barrying any objections,
> I'm thinking to commit this patch.
I'd suggest:
"to print one message each ..." -> "to print one message every ..."
"to print no progress ..." -> "not to print any progress ..."
I would not call "fprintf(stderr" twice in a row if I can call it once.
> While reviewing the patch, I found that current code allows space
> character to be specified in -I. That is, checkInitSteps() accepts
> space character. Why should we do this?
> Probably I understand why runInitSteps() needs to accept space character
> (because "v" in the specified string with -I is replaced with a space
> character when --no-vacuum option is given).
Yes, that is the reason, otherwise the string would have to be shifted.
> But I'm not sure why that's also necessary in checkInitSteps(). Instead,
> we should treat a space character as invalid in checkInitSteps()?
I think that it may break --no-vacuum, and I thought that there may be
other option which remove things, eventually. Also, having a NO-OP looks
ok to me.
--
Fabien.
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2019-11-05 21:29:16 | Re: deferrable FK constraints on partitioned rels |
Previous Message | Robert Haas | 2019-11-05 21:16:14 | Re: [HACKERS] WAL logging problem in 9.4.3? |