Re: parallel.c is not marked as test covered

On Thu, Jun 16, 2016 at 12:46 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Wed, Jun 15, 2016 at 5:23 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)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.
>

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.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com