Re: pgsql: Fix several mistakes around parallel workers and client_encoding

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Robert Haas <rhaas(at)postgresql(dot)org>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Fix several mistakes around parallel workers and client_encoding
Date: 2016-06-30 23:52:13
Message-ID: E5094B99-E083-480F-9E0D-8C4AF688184A@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Actually, a significant part of this was derived from Peter's patch. I should have credited him as a co-author as well as a reviewer. I apologize for the oversight.

...Robert

> On Jun 30, 2016, at 6:46 PM, Robert Haas <rhaas(at)postgresql(dot)org> wrote:
>
> Fix several mistakes around parallel workers and client_encoding.
>
> Previously, workers sent data to the leader using the client encoding.
> That mostly worked, but the leader the converted the data back to the
> server encoding. Since not all encoding conversions are reversible,
> that could provoke failures. Fix by using the database encoding for
> all communication between worker and leader.
>
> Also, while temporary changes to GUC settings, as from the SET clause
> of a function, are in general OK for parallel query, changing
> client_encoding this way inside of a parallel worker is not OK.
> Previously, that would have confused the leader; with these changes,
> it would not confuse the leader, but it wouldn't do anything either.
> So refuse such changes in parallel workers.
>
> Also, the previous code naively assumed that when it received a
> NotifyResonse from the worker, it could pass that directly back to the
> user. But now that worker-to-leader communication always uses the
> database encoding, that's clearly no longer correct - though,
> actually, the old way was always broken for V2 clients. So
> disassemble and reconstitute the message instead.
>
> Issues reported by Peter Eisentraut. Patch by me, reviewed by
> Peter Eisentraut.
>
> Branch
> ------
> master
>
> Details
> -------
> http://git.postgresql.org/pg/commitdiff/10c0558ffefcd12bf1d3dc35587eba41d1ce4571
>
> Modified Files
> --------------
> src/backend/access/transam/parallel.c | 18 +++++++++++++++++-
> src/backend/commands/async.c | 5 +----
> src/backend/commands/variable.c | 24 ++++++++++++++++++++++++
> src/backend/libpq/pqformat.c | 30 ++++++++++++++++++++++++++++++
> src/backend/libpq/pqmq.c | 2 +-
> src/include/commands/async.h | 4 ++++
> src/include/libpq/pqformat.h | 1 +
> 7 files changed, 78 insertions(+), 6 deletions(-)
>
>
> --
> Sent via pgsql-committers mailing list (pgsql-committers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-committers

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Noah Misch 2016-07-01 03:51:18 Re: pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <
Previous Message Robert Haas 2016-06-30 22:46:39 pgsql: Fix several mistakes around parallel workers and client_encoding