Re: dblink performance regression

Joe Conway <mail(at)joeconway(dot)com> writes:
> Apparently setting client encoding is an expensive operation.

Yeah, it requires sending a command to the remote end. I'm not
sure if it'd be a good idea for PQsetClientEncoding to try to
skip the operation when the client encoding is already the
right thing. The given name might not be spelled canonically,
for one thing.

> My proposed fix (diff attached) is to simply skip setting client
> encoding in the case where it already matches server side encoding.

I have to think that we shouldn't need a string compare to figure out
if one integer encoding ID is equal to another. Why not just compare
PQclientEncoding(conn) to GetDatabaseEncoding()? Otherwise, seems
reasonable.

Actually ... there's an interesting conflict here. What if libpq hasn't
got the same encoding ID number assignments as the backend? I suspect
that the proposed code is actually wrong, or at least risks being wrong,
because it's far from clear whether the linker will resolve
pg_encoding_to_char() as being libpq's version or the backend's version.
The former choice would cause things to work, as long as the textual names
agree, but the latter choice would be just as broken as if we'd simply
compared the integer IDs.

I seem to remember that at some point we realized that the encoding ID
assignments are part of libpq's ABI and so can't practically be changed
ever, so the above may be moot. Even so, I think it's a bad idea to be
depending on pg_encoding_to_char() here, given the ambiguity in what it
references. It would be unsurprising to get build-time or run-time
failures on pickier platforms, as a consequence of that ambiguity. So
I'd still recommend comparing integer IDs as above, rather than this.

regards, tom lane