Re: parallel.c is not marked as test covered
| От | Amit Kapila |
|---|---|
| Тема | Re: parallel.c is not marked as test covered |
| Дата | |
| Msg-id | CAA4eK1KZU80eVCJLesi8Gd3ZCDL5B3NoAzJ7h9T=iDS4h4=hyg@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: parallel.c is not marked as test covered (Robert Haas <robertmhaas@gmail.com>) |
| Ответы |
Re: parallel.c is not marked as test covered
|
| Список | pgsql-hackers |
On Thu, Jun 16, 2016 at 12:46 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Jun 15, 2016 at 5:23 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >> > Considering above analysis is correct, we have below options:
> >> > a. Modify the test such that it actually generates an error and to hide
> >> > the
> >> > context, we can exception block and raise some generic error.
> >> > b. Modify the test such that it actually generates an error and to hide
> >> > the
> >> > context, we can use force_parallel_mode = regress;
> >>
> >> Either of those sounds okay. No need to raise a generic error; one can
> >> raise
> >> SQLERRM to keep the main message and not the context. I lean toward (a)
> >> so we
> >> have nonzero test coverage of force_parallel_mode=on.
> >
> > Patch implementing option (a) attached with this mail.
>
> OK, committed.
>
> On Wed, Jun 15, 2016 at 5:23 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >> > Considering above analysis is correct, we have below options:
> >> > a. Modify the test such that it actually generates an error and to hide
> >> > the
> >> > context, we can exception block and raise some generic error.
> >> > b. Modify the test such that it actually generates an error and to hide
> >> > the
> >> > context, we can use force_parallel_mode = regress;
> >>
> >> Either of those sounds okay. No need to raise a generic error; one can
> >> raise
> >> SQLERRM to keep the main message and not the context. I lean toward (a)
> >> so we
> >> have nonzero test coverage of force_parallel_mode=on.
> >
> > Patch implementing option (a) attached with this mail.
>
> OK, committed.
Thanks.
>I also changed "select" to "perform" per your
> analysis.
oops, it seems I have forgotten to make that change in patch.
>
> I wonder if we need to revisit the choices I made inside
> PL/pgsql and see why CURSOR_OPT_PARALLEL_OK is not being set here.
>
> analysis.
oops, it seems I have forgotten to make that change in patch.
>
> I wonder if we need to revisit the choices I made inside
> PL/pgsql and see why CURSOR_OPT_PARALLEL_OK is not being set here.
>
exec_stmt_execsql() is used to execute SQL statements insider plpgsql which includes dml statements as well, so probably you wanted to play safe by not allowing parallel option from that place. However, I think there shouldn't be a problem in using CURSOR_OPT_PARALLEL_OK from this place as we have a check in standard_planner which will take care of whether to choose parallel mode or not for a particular statement. If you want, I can do more detailed analysis and prepare a patch.
В списке pgsql-hackers по дате отправления: