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.
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.
