Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0ae5b76

Browse files
committed
Clean up handling of client_encoding GUC in parallel workers.
The previous coding here threw an error from assign_client_encodingif it was invoked in a parallel worker. That's a very fundamentalviolation of the GUC hook API: assign hooks must not throw errors.The place to complain is in the check hook, so move the test tothere, and use the regular check-hook API (ie return false) toreport it.The reason this coding is a problem is that it breaks GUC rollback,which may occur after we leave InitializingParallelWorker state.That case seems not actually reachable before now, but commitf5f30c2 made it reachable, so we need to fix this before thatcan be un-reverted.In passing, improve the commentary in ParallelWorkerMain, andadd a check for failure of SetClientEncoding. That's anothercase that can't happen now but might become possible afterforeseeable code rearrangements (notably, if the shortcut ofskipping PrepareClientEncoding stops being OK).Discussion:https://postgr.es/m/18545-feba138862f19aaa@postgresql.org
1 parent8928817 commit0ae5b76

File tree

2 files changed

+32
-23
lines changed

2 files changed

+32
-23
lines changed

‎src/backend/access/transam/parallel.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,9 +1398,13 @@ ParallelWorkerMain(Datum main_arg)
13981398

13991399
/*
14001400
* Set the client encoding to the database encoding, since that is what
1401-
* the leader will expect.
1401+
* the leader will expect. (We're cheating a bit by not calling
1402+
* PrepareClientEncoding first. It's okay because this call will always
1403+
* result in installing a no-op conversion. No error should be possible,
1404+
* but check anyway.)
14021405
*/
1403-
SetClientEncoding(GetDatabaseEncoding());
1406+
if (SetClientEncoding(GetDatabaseEncoding())<0)
1407+
elog(ERROR,"SetClientEncoding(%d) failed",GetDatabaseEncoding());
14041408

14051409
/*
14061410
* Load libraries that were loaded by original backend. We want to do

‎src/backend/commands/variable.c

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,24 @@ check_client_encoding(char **newval, void **extra, GucSource source)
690690
/* Get the canonical name (no aliases, uniform case) */
691691
canonical_name=pg_encoding_to_char(encoding);
692692

693+
/*
694+
* Parallel workers send data to the leader, not the client. They always
695+
* send data using the database encoding; therefore, we should never
696+
* actually change the client encoding in a parallel worker. However,
697+
* during parallel worker startup, we want to accept the leader's
698+
* client_encoding setting so that anyone who looks at the value in the
699+
* worker sees the same value that they would see in the leader. A change
700+
* other than during startup, for example due to a SET clause attached to
701+
* a function definition, should be rejected, as there is nothing we can
702+
* do inside the worker to make it take effect.
703+
*/
704+
if (IsParallelWorker()&& !InitializingParallelWorker)
705+
{
706+
GUC_check_errcode(ERRCODE_INVALID_TRANSACTION_STATE);
707+
GUC_check_errdetail("Cannot change \"client_encoding\" during a parallel operation.");
708+
return false;
709+
}
710+
693711
/*
694712
* If we are not within a transaction then PrepareClientEncoding will not
695713
* be able to look up the necessary conversion procs. If we are still
@@ -700,11 +718,15 @@ check_client_encoding(char **newval, void **extra, GucSource source)
700718
* It seems like a bad idea for client_encoding to change that way anyhow,
701719
* so we don't go out of our way to support it.
702720
*
721+
* In a parallel worker, we might as well skip PrepareClientEncoding since
722+
* we're not going to use its results.
723+
*
703724
* Note: in the postmaster, or any other process that never calls
704725
* InitializeClientEncoding, PrepareClientEncoding will always succeed,
705726
* and so will SetClientEncoding; but they won't do anything, which is OK.
706727
*/
707-
if (PrepareClientEncoding(encoding)<0)
728+
if (!IsParallelWorker()&&
729+
PrepareClientEncoding(encoding)<0)
708730
{
709731
if (IsTransactionState())
710732
{
@@ -758,28 +780,11 @@ assign_client_encoding(const char *newval, void *extra)
758780
intencoding=*((int*)extra);
759781

760782
/*
761-
*Parallel workers send data to the leader, notthe client. They always
762-
*send data using the database encoding.
783+
*In a parallel worker, we never overridethe client encoding that was
784+
*set by ParallelWorkerMain().
763785
*/
764786
if (IsParallelWorker())
765-
{
766-
/*
767-
* During parallel worker startup, we want to accept the leader's
768-
* client_encoding setting so that anyone who looks at the value in
769-
* the worker sees the same value that they would see in the leader.
770-
*/
771-
if (InitializingParallelWorker)
772-
return;
773-
774-
/*
775-
* A change other than during startup, for example due to a SET clause
776-
* attached to a function definition, should be rejected, as there is
777-
* nothing we can do inside the worker to make it take effect.
778-
*/
779-
ereport(ERROR,
780-
(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
781-
errmsg("cannot change \"client_encoding\" during a parallel operation")));
782-
}
787+
return;
783788

784789
/* We do not expect an error if PrepareClientEncoding succeeded */
785790
if (SetClientEncoding(encoding)<0)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp