|From:||Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>|
|To:||Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>|
|Cc:||Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, pgsql-hackers(at)postgresql(dot)org|
|Subject:||Re: pgbench: using prepared BEGIN statement in a pipeline could cause an error|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On Mon, 19 Jul 2021 10:51:36 +0900
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp> wrote:
> Hello Fabien,
> On Sat, 17 Jul 2021 07:03:01 +0200 (CEST)
> Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
> > Hello Yugo-san,
> > > [...] One way to avoid these errors is to send Parse messages before
> > > pipeline mode starts. I attached a patch to fix to prepare commands at
> > > starting of a script instead of at the first execution of the command.
> > > What do you think?
> > ISTM that moving prepare out of command execution is a good idea, so I'm
> > in favor of this approach: the code is simpler and cleaner.
> > ISTM that a minor impact is that the preparation is not counted in the
> > command performance statistics. I do not think that it is a problem, even
> > if it would change detailed results under -C -r -M prepared.
> I agree with you. Currently, whether prepares are sent in pipeline mode or
> not depends on whether the first SQL command is placed between \startpipeline
> and \endpipeline regardless whether other commands are executed in pipeline
> or not. ISTM, this behavior would be not intuitive for users. Therefore,
> I think preparing all statements not using pipeline mode is not problem for now.
> If some users would like to send prepares in pipeline, I think it would be
> better to provide more simple and direct way. For example, we prepare statements
> in pipeline if the user use an option, or if the script has at least one
> \startpipeline in their pipeline. Maybe, --pipeline option is useful for users
> who want to use pipeline mode for all queries in scirpts including built-in ones.
> However, these features seems to be out of the patch proposed in this thread.
> > Patch applies & compiles cleanly, global & local make check ok. However
> > the issue is not tested. I think that the patch should add a tap test case
> > for the problem being addressed.
> Ok. I'll add a tap test to confirm the error I found is avoidable.
> > I'd suggest to move the statement preparation call in the
> > CSTATE_CHOOSE_SCRIPT case.
> I thought so at first, but I noticed we cannot do it at least if we are
> using -C because the connection may not be established in the
> CSTATE_CHOOSE_SCRIPT state.
> > In comments: not yet -> needed.
> Thanks. I'll fix it.
I attached the updated patch v2, which includes a comment fix and a TAP test.
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>
|Next Messagefirstname.lastname@example.org||2021-07-21 02:15:14||add 'noError' to euc_tw_and_big5.c|
|Previous Message||James Coleman||2021-07-21 01:39:14||Re: [PATCH] Use optimized single-datum tuplesort in ExecSort|