From b922e417660fa37ed9f2b90edf9a6d59c1b3455e Mon Sep 17 00:00:00 2001 From: tgl Date: Thu, 2 Apr 2009 17:30:53 +0000 Subject: [PATCH] Fix SetClientEncoding() to maintain a cache of previously selected encoding conversion functions. This allows transaction rollback to revert to a previous client_encoding setting without doing fresh catalog lookups. I believe that this explains and fixes the recent report of "failed to commit client_encoding" failures. This bug is present in 8.3.x, but it doesn't seem prudent to back-patch the fix, at least not till it's had some time for field testing in HEAD. In passing, remove SetDefaultClientEncoding(), which was used nowhere. --- src/backend/utils/mb/mbutils.c | 208 +++++++++++++++++++++++++---------------- src/include/mb/pg_wchar.h | 1 - 2 files changed, 125 insertions(+), 84 deletions(-) diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c index bf66321134..661545801b 100644 --- a/src/backend/utils/mb/mbutils.c +++ b/src/backend/utils/mb/mbutils.c @@ -1,7 +1,7 @@ /* * This file contains public functions for conversion between - * client encoding and server internal encoding. - * (currently mule internal code (mic) is used) + * client encoding and server (database) encoding. + * * Tatsuo Ishii * * $PostgreSQL$ @@ -28,24 +28,38 @@ #define MAX_CONVERSION_GROWTH 4 /* - * We handle for actual FE and BE encoding setting encoding-identificator - * and encoding-name too. It prevent searching and conversion from encoding - * to encoding name in getdatabaseencoding() and other routines. + * We maintain a simple linked list caching the fmgr lookup info for the + * currently selected conversion functions, as well as any that have been + * selected previously in the current session. (We remember previous + * settings because we must be able to restore a previous setting during + * transaction rollback, without doing any fresh catalog accesses.) + * + * Since we'll never release this data, we just keep it in TopMemoryContext. */ -static pg_enc2name *ClientEncoding = &pg_enc2name_tbl[PG_SQL_ASCII]; -static pg_enc2name *DatabaseEncoding = &pg_enc2name_tbl[PG_SQL_ASCII]; +typedef struct ConvProcInfo +{ + int s_encoding; /* server and client encoding IDs */ + int c_encoding; + FmgrInfo to_server_info; /* lookup info for conversion procs */ + FmgrInfo to_client_info; +} ConvProcInfo; + +static List *ConvProcList = NIL; /* List of ConvProcInfo */ /* - * Caches for conversion function info. These values are allocated in - * MbProcContext. That context is a child of TopMemoryContext, - * which allows these values to survive across transactions. See - * SetClientEncoding() for more details. + * These variables point to the currently active conversion functions, + * or are NULL when no conversion is needed. */ -static MemoryContext MbProcContext = NULL; static FmgrInfo *ToServerConvProc = NULL; static FmgrInfo *ToClientConvProc = NULL; /* + * These variables track the currently selected FE and BE encodings. + */ +static pg_enc2name *ClientEncoding = &pg_enc2name_tbl[PG_SQL_ASCII]; +static pg_enc2name *DatabaseEncoding = &pg_enc2name_tbl[PG_SQL_ASCII]; + +/* * During backend startup we can't set client encoding because we (a) * can't look up the conversion functions, and (b) may not know the database * encoding yet either. So SetClientEncoding() just accepts anything and @@ -70,11 +84,7 @@ int SetClientEncoding(int encoding, bool doit) { int current_server_encoding; - Oid to_server_proc, - to_client_proc; - FmgrInfo *to_server; - FmgrInfo *to_client; - MemoryContext oldcontext; + ListCell *lc; if (!PG_VALID_FE_ENCODING(encoding)) return -1; @@ -101,79 +111,117 @@ SetClientEncoding(int encoding, bool doit) ClientEncoding = &pg_enc2name_tbl[encoding]; ToServerConvProc = NULL; ToClientConvProc = NULL; - if (MbProcContext) - MemoryContextReset(MbProcContext); } return 0; } - /* - * If we're not inside a transaction then we can't do catalog lookups, so - * fail. After backend startup, this could only happen if we are - * re-reading postgresql.conf due to SIGHUP --- so basically this just - * constrains the ability to change client_encoding on the fly from - * postgresql.conf. Which would probably be a stupid thing to do anyway. - */ - if (!IsTransactionState()) - return -1; + if (IsTransactionState()) + { + /* + * If we're in a live transaction, it's safe to access the catalogs, + * so look up the functions. We repeat the lookup even if the info + * is already cached, so that we can react to changes in the contents + * of pg_conversion. + */ + Oid to_server_proc, + to_client_proc; + ConvProcInfo *convinfo; + MemoryContext oldcontext; + + to_server_proc = FindDefaultConversionProc(encoding, + current_server_encoding); + if (!OidIsValid(to_server_proc)) + return -1; + to_client_proc = FindDefaultConversionProc(current_server_encoding, + encoding); + if (!OidIsValid(to_client_proc)) + return -1; - /* - * Look up the conversion functions. - */ - to_server_proc = FindDefaultConversionProc(encoding, - current_server_encoding); - if (!OidIsValid(to_server_proc)) - return -1; - to_client_proc = FindDefaultConversionProc(current_server_encoding, - encoding); - if (!OidIsValid(to_client_proc)) - return -1; + /* + * Done if not wanting to actually apply setting. + */ + if (!doit) + return 0; - /* - * Done if not wanting to actually apply setting. - */ - if (!doit) - return 0; + /* + * Load the fmgr info into TopMemoryContext (could still fail here) + */ + convinfo = (ConvProcInfo *) MemoryContextAlloc(TopMemoryContext, + sizeof(ConvProcInfo)); + convinfo->s_encoding = current_server_encoding; + convinfo->c_encoding = encoding; + fmgr_info_cxt(to_server_proc, &convinfo->to_server_info, + TopMemoryContext); + fmgr_info_cxt(to_client_proc, &convinfo->to_client_info, + TopMemoryContext); + + /* Attach new info to head of list */ + oldcontext = MemoryContextSwitchTo(TopMemoryContext); + ConvProcList = lcons(convinfo, ConvProcList); + MemoryContextSwitchTo(oldcontext); - /* Before loading the new fmgr info, remove the old info, if any */ - ToServerConvProc = NULL; - ToClientConvProc = NULL; - if (MbProcContext != NULL) - { - MemoryContextReset(MbProcContext); + /* + * Everything is okay, so apply the setting. + */ + ClientEncoding = &pg_enc2name_tbl[encoding]; + ToServerConvProc = &convinfo->to_server_info; + ToClientConvProc = &convinfo->to_client_info; + + /* + * Remove any older entry for the same encoding pair (this is just + * to avoid memory leakage). + */ + foreach(lc, ConvProcList) + { + ConvProcInfo *oldinfo = (ConvProcInfo *) lfirst(lc); + + if (oldinfo == convinfo) + continue; + if (oldinfo->s_encoding == convinfo->s_encoding && + oldinfo->c_encoding == convinfo->c_encoding) + { + ConvProcList = list_delete_ptr(ConvProcList, oldinfo); + pfree(oldinfo); + break; /* need not look further */ + } + } + + return 0; /* success */ } else { /* - * This is the first time through, so create the context. Make it a - * child of TopMemoryContext so that these values survive across - * transactions. + * If we're not in a live transaction, the only thing we can do + * is restore a previous setting using the cache. This covers all + * transaction-rollback cases. The only case it might not work for + * is trying to change client_encoding on the fly by editing + * postgresql.conf and SIGHUP'ing. Which would probably be a stupid + * thing to do anyway. */ - MbProcContext = AllocSetContextCreate(TopMemoryContext, - "MbProcContext", - ALLOCSET_SMALL_MINSIZE, - ALLOCSET_SMALL_INITSIZE, - ALLOCSET_SMALL_MAXSIZE); - } - - /* Load the fmgr info into MbProcContext */ - oldcontext = MemoryContextSwitchTo(MbProcContext); - to_server = palloc(sizeof(FmgrInfo)); - to_client = palloc(sizeof(FmgrInfo)); - fmgr_info(to_server_proc, to_server); - fmgr_info(to_client_proc, to_client); - MemoryContextSwitchTo(oldcontext); + foreach(lc, ConvProcList) + { + ConvProcInfo *oldinfo = (ConvProcInfo *) lfirst(lc); - ClientEncoding = &pg_enc2name_tbl[encoding]; - ToServerConvProc = to_server; - ToClientConvProc = to_client; + if (oldinfo->s_encoding == current_server_encoding && + oldinfo->c_encoding == encoding) + { + if (doit) + { + ClientEncoding = &pg_enc2name_tbl[encoding]; + ToServerConvProc = &oldinfo->to_server_info; + ToClientConvProc = &oldinfo->to_client_info; + } + return 0; + } + } - return 0; + return -1; /* it's not cached, so fail */ + } } /* * Initialize client encoding if necessary. - * called from InitPostgres() once during backend starting up. + * called from InitPostgres() once during backend startup. */ void InitializeClientEncoding(void) @@ -196,7 +244,8 @@ InitializeClientEncoding(void) } /* - * returns the current client encoding */ + * returns the current client encoding + */ int pg_get_client_encoding(void) { @@ -511,10 +560,9 @@ pg_server_to_client(const char *s, int len) /* * Perform default encoding conversion using cached FmgrInfo. Since * this function does not access database at all, it is safe to call - * outside transactions. Explicit setting client encoding required - * before calling this function. Otherwise no conversion is - * performed. -*/ + * outside transactions. If the conversion has not been set up by + * SetClientEncoding(), no conversion is performed. + */ static char * perform_default_encoding_conversion(const char *src, int len, bool is_client_to_server) { @@ -919,12 +967,6 @@ pg_bind_textdomain_codeset(const char *domainname, int encoding) #endif } -void -SetDefaultClientEncoding(void) -{ - ClientEncoding = &pg_enc2name_tbl[GetDatabaseEncoding()]; -} - int GetDatabaseEncoding(void) { diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h index 76322c9e36..3525299b2a 100644 --- a/src/include/mb/pg_wchar.h +++ b/src/include/mb/pg_wchar.h @@ -383,7 +383,6 @@ extern size_t wchar2char(char *to, const wchar_t *from, size_t tolen); extern size_t char2wchar(wchar_t *to, size_t tolen, const char *from, size_t fromlen); #endif -extern void SetDefaultClientEncoding(void); extern int SetClientEncoding(int encoding, bool doit); extern void InitializeClientEncoding(void); extern int pg_get_client_encoding(void); -- 2.11.4.GIT