Re: parallel workers and client encoding

On 6/27/16 5:37 PM, Robert Haas wrote:
> Please find attached an a patch for a proposed alternative approach.
> This does the following:
>
> 1. When the client_encoding GUC is changed in the worker,
> SetClientEncoding() is not called.

I think this could be a problem, because then the client encoding in the
background worker process is inherited from the postmaster, which could
in theory be anything. I think you need to set it at least once to the
correct value.

> Thus, GetClientEncoding() in the
> worker always returns the database encoding, regardless of how the
> client encoding is set. This is better than your approach of calling
> SetClientEncoding() during worker startup, I think, because the worker
> could call a parallel-safe function with SET clause, and one of the
> GUCs temporarily set could be client_encoding. That would be stupid,
> but somebody could do it.

I think if we're worried about this, then this should be an error, but
that's a minor concern.

I realize that we don't have a good mechanism in the GUC code to
distinguish these two situations.

Then again, this shouldn't be so much different in concept from the
restoring of GUC variables in the EXEC_BACKEND case. There is special
code in set_config_option() to bypass some of the rules in that case.
RestoreGUCState() should be able to get the same sort of pass.

Also, set_config_option() knows something about what settings are
allowed in a parallel worker, so I wonder if setting client_encoding
would even work in spite of that?

> 2. A new function pq_getmsgrawstring() is added. This is like
> pq_getmsgstring() but it does no encoding conversion.
> pq_parse_errornotice() is changed to use pq_getmsgrawstring() instead
> of pq_getmsgstring(). Because of (1), when the leader receives an
> ErrorResponse or NoticeResponse from the worker, it will not have been
> subject to encoding conversion; because of this item, the leader will
> not try to convert it either when initially parsing it. So the extra
> encoding round-trip is avoided.

I like that.

> 3. The changes for NotifyResponse which you proposed are included
> here, but with the modification that pq_getmsgrawstring() is used.

and that.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services