On Tue, Feb 22, 2022 at 07:10:44PM -0500, Wietse Venema wrote: > There are two #ifdef SNAPSHOT blocks. > > - The one in dict_pgsql_lookup() returns not found when SMTPUTF8 > is enabled, but a query is not valid UTF8. > > - The one in plpgsql_connect_single() sets the PgSQL client encoding > to UTF8, and if SNAPSHOT is not defined, leaves it at LATIN1 > encoding which is what was used historically. > > How do we avoid unexpected breakages in a stable release? > > I suggest that we leave the encoding at LATIN1 when SMTPUTF8 is > disabled. That is historical behavior and that minimizes the risk > of breakages. > > We can then use the UTF8 encoding when SMTPUTF8 is enabled. It is > the only encoding that makes sense with SMTPUTF8.
Sure, but there is some small risk that a site that enables UTF8 nevertheless is currently using a database with a server-side encoding of LATIN1 and somehow avoiding actual use of UTF8 lookup keys. This risk is probably quite modest. Introducing a dictionary configuration keyword is fairly simple, with the proposed behaviour as a default, but then also a mechanism to disable UTF8 if desired. --- src/global/dict_pgsql.c +++ src/global/dict_pgsql.c @@ -207,6 +207,7 @@ typedef struct { char *result_format; void *ctx; int expansion_limit; + int utf8_enable; char *username; char *password; char *dbname; @@ -722,6 +723,7 @@ static void pgsql_parse_config(DICT_PGSQL *dict_pgsql, const char *pgsqlcf) dict_pgsql->password = cfg_get_str(p, "password", "", 0, 0); dict_pgsql->dbname = cfg_get_str(p, "dbname", "", 1, 0); dict_pgsql->result_format = cfg_get_str(p, "result_format", "%s", 1, 0); + dict_pgsql->utf8_enable = cfg_get_bool(p, "utf8_enable", 1); /* * XXX: The default should be non-zero for safety, but that is not The new dictionary parameter can then be used a guard to conditionally suppress use of the UTF8 encoding in the PgSQL driver. But if that's a bridge too far for a stable release, so be it. (Of course the documentation would then have to describe the parater as applicable from a certain patch level and up, IIRC there were some precedents for this sort of emergency UI expansion, though none recently). -- Viktor.