Re: parallel.c is not marked as test covered

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, Clément Prévost <prevostclement(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: parallel.c is not marked as test covered
Date: 2016-06-14 20:12:10
Message-ID: CA+TgmobQSpF68eYELRg5WkH6Zz6=ajJU85C1JCCw83YZ01RyWQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 14, 2016 at 1:14 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Tue, Jun 14, 2016 at 12:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> FWIW, I follow all of your reasoning except this. If we believe that the
>>> parallel worker context line is useful, then it is a bug that plpgsql
>>> suppresses it. If we don't believe it's useful, then we should get
>>> rid of it. "Do nothing" is simply not a consistent stance here.
>
>> Well, if PL/pgsql suppresses context and nobody's complained about
>> that up until now, fixing it doesn't seem any more urgent than any
>> other bug we've had for N releases.
>
> 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.

The phrasing of the comment implies that (1) the behavior is desirable
at least for the purpose at hand and (2) the behavior is unrelated to
parallel query. However, it's surely possible that (2) is false and
(1) is not the consensus. Noah is usually pretty careful about this
sort of thing, but he might have made a mistake. Considering that, I
decide to take a look.

I wasn't able in a quick test to verify that this behavior doesn't
happen without parallel query. I also wasn't able to figure out
exactly why it was happening. I did verify that this statement
generates an error that is propagated to the leader. The expected
output looks like this:

ERROR: invalid input syntax for integer: "BAAAAA"
CONTEXT: SQL statement "select stringu1::int2 from tenk1 where unique1 = 1"
PL/pgSQL function inline_code_block line 5 at SQL statement

The first line of context begins with "SQL statement" and I believe
that's coming from spi.c. The second line of the context seems to
come from PL/pgsql. But what accounts for the failure of the "parallel
query" line to appear in the context message? This could be explained
by _SPI_error_callback flushing the existing context in its handler,
but it doesn't seem to do that. I guess this needs some more looking
at.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2016-06-14 20:12:46 Re: Prepared statements and generic plans
Previous Message Jim Nasby 2016-06-14 20:05:01 Re: 10.0