Re: parallel.c is not marked as test covered - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: parallel.c is not marked as test covered
Date
Msg-id CAA4eK1Jv-e3reqkZSpE3fBSJfQEFnYLNpC8HwDB_uESEXHboVA@mail.gmail.com
Whole thread Raw
In response to Re: parallel.c is not marked as test covered  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: parallel.c is not marked as test covered  (Noah Misch <noah@leadboat.com>)
List pgsql-hackers
On Wed, Jun 15, 2016 at 1:42 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Tue, Jun 14, 2016 at 1:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > I have not dug into the code enough to find out exactly what's happening
> > in Peter's complaint, but it seems like it would be a good idea to find
> > that out before arguing along these lines.  It seems entirely likely
> > to me that this is somehow parallel-query-specific.  Even if it isn't,
> > I don't buy your argument.  Adding a new case in which context is
> > suppressed is a perfectly reasonable basis for thinking that an old
> > bug has acquired new urgency.
>
> OK.  I find this whole thing slightly puzzling because Noah wrote this
> in the test file:
>
>   -- Provoke error in worker.  The original message CONTEXT contains a worker
>   -- PID that must be hidden in the test output.  PL/pgSQL conveniently
>   -- substitutes its own CONTEXT.
>

I have done analysis of this issue and found out that as written, test will never generate any sort of parallel plan which is an expected behaviour as per design.  The above error is generated in backend and that's why no Context: parallel worker, PID <pid_of_worker>.  Now, one might wonder why in-spite of setting force_parallel_mode = 1, this test won't generate a parallel plan. The reason for the same is that for SQL statements in plpgsql (PLPGSQL_STMT_EXECSQL), parallelModeOK will be false (we don't use CURSOR_OPT_PARALLEL_OK for such statements).  For unsafe statements (parallelModeOK=false), we don't force parallel plans even force_parallel_mode is enabled.

In short, this test doesn't serve it's purpose which is to generate an error from worker.  A modified version as below can generate such an error and the same is displayed in context as well.

do $$begin
  Perform stringu1::int2 from tenk1 where unique1 = 1;
end$$;

ERROR:  invalid input syntax for integer: "BAAAAA"
CONTEXT:  parallel worker, PID 4460
SQL statement "SELECT stringu1::int2 from tenk1 where unique1 = 1"
PL/pgSQL function inline_code_block line 2 at PERFORM

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;
c. Remove this test and then think of a better way to exercise error path in worker. 

Thoughts?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Kyotaro HORIGUCHI
Date:
Subject: Re: [BUG] pg_basebackup from disconnected standby fails
Next
From: Noah Misch
Date:
Subject: Re: parallel.c is not marked as test covered