Hello Fabien,
On Sat, 17 Jul 2021 07:03:01 +0200 (CEST)
Fabien COELHO <coelho@cri.ensmp.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.
Regards,
Yugo Nagata
--
Yugo NAGATA <nagata@sraoss.co.jp>
