Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\))
Subject: Re: BUG #15726: parallel queries failed ERROR: invalid name syntax
 CONTEXT: parallel worker
From: Daniel Gustafsson <dgustafsson@pivotal.io>
In-Reply-To: 
 <CA+hUKGJ=nTwC750vLGtQUGXFHFKxFF31=Qwn=Pd70S4jZanA2g@mail.gmail.com>
Date: Thu, 11 Apr 2019 10:59:41 +0200
Cc: Tom Lane <tgl@sss.pgh.pa.us>, ANASTACIO Tiago <tiago_anastacio@yahoo.fr>,
        Peter Geoghegan <pg@bowt.ie>,
        PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <C6B09DDC-FDDA-466D-AFBD-F8C966CD97C1@pivotal.io>
References: <15726-6d67e4fa14f027b3@postgresql.org>
 <CA+hUKG+BcXXqqRb1JNbUXYKyNXzyyOqvebYN+9DGqGY6UPtLQQ@mail.gmail.com>
 <CA+hUKGKJ-amPMuwFhgc5T6UH=Wcn3CG3gmowmkYqTo2WsQJM4A@mail.gmail.com>
 <CA+hUKGLdzBsa_SZajU8J9JDSHzdU6+LVq1g6bUsekiWBpjVDNQ@mail.gmail.com>
 <CAH2-Wzkif1R6imAhgcRv1cXqomYBDez9g1rKAhDixiEi7b2KOg@mail.gmail.com>
 <1831482307.24522245.1554199154256@mail.yahoo.com>
 <CA+hUKGLvwuKEf_dk-zZANRLGfT9eO5hbKB_+KYeb-Uef_Q7G-g@mail.gmail.com>
 <29888.1554218381@sss.pgh.pa.us>
 <CA+hUKGJ=nTwC750vLGtQUGXFHFKxFF31=Qwn=Pd70S4jZanA2g@mail.gmail.com>
To: Thomas Munro <thomas.munro@gmail.com>
Precedence: bulk

> On 2 Apr 2019, at 21:54, Thomas Munro <thomas.munro@gmail.com> wrote:
>=20
> On Wed, Apr 3, 2019 at 4:19 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Thomas Munro <thomas.munro@gmail.com> writes:
>>> On Tue, Apr 2, 2019 at 10:59 PM ANASTACIO Tiago
>>> <tiago_anastacio@yahoo.fr> wrote:
>>>> A pity background worker are no more verbose (DEBUG5) than:
>>>> 2019-04-02 09:42:38.993 GMT [17654] ERROR:  invalid name syntax
>>=20
>>> I agree, it would be nice to improve that.  It seems to be a general
>>> problem with any GUC that requires a transaction to check: parallel
>>> workers will force that, but you might not understand the error
>>> because it comes with no context.  That gives me an idea: perhaps we
>>> should set the elog() context while restoring GUCs, so that it is
>>> displayed in the error output somehow.
>>=20
>> Yeah, something like
>>=20
>> CONTEXT: while setting parameter guc_variable_name to "some value"
>>=20
>> would likely make this a whole lot more comprehensible.  And I think
>> it could probably be done in a fairly centralized way using an error
>> callback.
>=20
> Would you want to do it for GUC setting in general (for example using
> the SET command), or just in this GUC-restore-for-parallel-query case?
> Here's a 10 minute attempt at the latter.

I think only doing it for the latter makes sense, when invoking the SET =
command
it=E2=80=99s more reasonable to associate the ERROR with the performed =
action.

> postgres=3D# set force_parallel_mode =3D 'on';
> SET
> postgres=3D# select 42;
> psql: ERROR:  invalid name syntax
> CONTEXT:  while setting parameter "default_text_search_config" to ""
> parallel worker

This makes it a lot better, but I wonder if it needs even more =
clarification to
be easily understandable by users not familiar with how parallel workers =
are
initiated.  I=E2=80=99m thinking we should add =E2=80=9Cduring worker =
setup=E2=80=9D or something
similar to make it even clearer, albeit more verbose.

> It gets a bit repetitive in some other cases where it was already
> reporting the name and value, though:
> postgres=3D# select 42;
> psql: ERROR:  invalid value for parameter =
"default_text_search_config": "foo"
> CONTEXT:  while setting parameter "default_text_search_config" to =
"foo"
> parallel worker

It is indeed repetetive, but I=E2=80=99m not sure it=E2=80=99s worth =
addressing that.
Consistency in error messaging seems more important in this case.

cheers ./daniel=