Re: PQclientEncoding() returns -1, resulting in possible assertion failure in psql

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Geoghegan <pg(at)heroku(dot)com>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: PQclientEncoding() returns -1, resulting in possible assertion failure in psql
Date: 2013-12-08 18:56:44
Message-ID: 15210.1386529004@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

I wrote:
> Peter Geoghegan <pg(at)heroku(dot)com> writes:
>> If the call to PQclientEncoding() from processSQLNamePattern() (plus
>> probably from other places) gets an encoding of -1, (called when we
>> "\df+ somefunc" after the server is gone - you may have to do it
>> twice), that can be passed down to PQmblen(), which will in turn pass
>> that value to pg_encoding_mblen(), which doesn't expect it.

> A simple fix might be to return SQL_ASCII not -1. I don't think the
> -1 behavior is documented or relied on anywhere, is it?

I looked at this a bit more closely and it seems to be a legitimate
fix. I would also suggest that we remove the connection status check;
there is no very good reason to forget what we knew about the encoding
the moment the connection drops. So something like

int
PQclientEncoding(const PGconn *conn)
{
- if (!conn || conn->status != CONNECTION_OK)
- return -1;
+ if (!conn)
+ return PG_SQL_ASCII;
return conn->client_encoding;
}

A compromise that would probably fix the problem for psql, if not
other places, would be to just remove the status check (ie, first
line of above diff but not second). That might be the thing to
do if people are afraid to change the behavior this much in back
branches. I would argue however that the documentation nowhere
suggests that PQclientEncoding can return a bogus encoding ID,
so this is more likely to be a bug fix than a new bug for other
programs as well. Also, it looks to me like there are probably
other corner cases anyway where you can get PG_SQL_ASCII rather
than the actual encoding, because that's how we initialize
conn->client_encoding ...

regards, tom lane

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Peter Geoghegan 2013-12-08 20:36:44 Re: PQclientEncoding() returns -1, resulting in possible assertion failure in psql
Previous Message Pavel Stehule 2013-12-08 18:54:57 Re: Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist