pgsql2shp fix for "XInvalid endian flag value encountered"

[dfuhry2]

pgsql2shp gives "Invalid endian flag value encountered." error on PostgreSQL 9.2 due to its expecting bytea_output to be in 'escape' format. At some point PostgreSQL changed the default bytea output format from 'escape' to 'hex'.

Attached patch sets server bytea_output to 'escape' while connecting, fixing the problem.

$ pgsql2shp ... Initializing... Done (postgis major version: 2). Output shape: Polygon Dumping: XInvalid endian flag value encountered.

[pramsey]

Trunk at r9756, thanks!

[mcayland]

NAK. This patch is broken and should be reverted because:

1) The default switch from 'escape' to 'hex' took place in PostgreSQL 9.0, so I'm not yet convinced that your diagnosis pointing at PostgreSQL 9.2 is correct

2) You've now broken dumping from older (< 9.0) PostgreSQL servers during a micro/patch revision

The symptoms suggest that your pgsql2shp client binary is linked against a pre-9.0 libpq somewhere else on your system, which is why libpq can't successfully interpret the 'hex' encoding being received from the server (we use libpq's internal PQescapeBytea() function rather than doing it ourselves).

In short, we need a better investigation of the problem and a revised patch to suit.

[robe]

Yah I'll double-check but I don't recall having issues dumping and viewing my shape file in ArcGIS, QGIS, or OpenJump. I'll have to double-check to make sure I didn't have an encoding setting on, but I don't think I did last I tried. Mark is right this bytea_output change happened in 9.0 not 9.2. So this could be possibly something else introduced in 9.2.

We should at the very least have in our make check routine a test that dumps out and then reloads the dumped shape file. I forget if any of our tests does that.

at anyrate, Paul you need to revert that patch since as Mark says -- it will break 8.4 support which we have to support in 2.0.

[pramsey]

Ok, reverted at r9760

[dfuhry2]

Linker information of my pgsql2shp executable:

$ ldd /usr/local/pgsql/bin/pgsql2shp
	linux-vdso.so.1 =>  (0x00007fff5d4b2000)
	liblwgeom-2.0.1SVN.so => /usr/local/lib/liblwgeom-2.0.1SVN.so (0x00007effe2e41000)
	libc.so.6 => /lib/libc.so.6 (0x00007effe2ae0000)
	libpq.so.5 => /usr/lib/libpq.so.5 (0x00007effe28b7000)
	libgeos_c.so.1 => /usr/local/lib/libgeos_c.so.1 (0x00007effe2691000)
	libproj.so.0 => /usr/lib/libproj.so.0 (0x00007effe244f000)
	/lib64/ld-linux-x86-64.so.2 (0x00007effe30a4000)
	libssl.so.0.9.8 => /usr/lib/libssl.so.0.9.8 (0x00007effe21f9000)
	libcrypto.so.0.9.8 => /usr/lib/libcrypto.so.0.9.8 (0x00007effe1e58000)
	libkrb5.so.3 => /usr/lib/libkrb5.so.3 (0x00007effe1b90000)
	libcom_err.so.2 => /lib/libcom_err.so.2 (0x00007effe198c000)
	libgssapi_krb5.so.2 => /usr/lib/libgssapi_krb5.so.2 (0x00007effe1757000)
	libcrypt.so.1 => /lib/libcrypt.so.1 (0x00007effe1520000)
	libldap_r-2.4.so.2 => /usr/lib/libldap_r-2.4.so.2 (0x00007effe12d4000)
	libpthread.so.0 => /lib/libpthread.so.0 (0x00007effe10b8000)
	libgeos-3.3.3.so => /usr/local/lib/libgeos-3.3.3.so (0x00007effe0d1a000)
	libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0x00007effe0a05000)
	libm.so.6 => /lib/libm.so.6 (0x00007effe0783000)
	libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x00007effe056d000)
	libdl.so.2 => /lib/libdl.so.2 (0x00007effe0368000)
	libz.so.1 => /usr/lib/libz.so.1 (0x00007effe0151000)
	libk5crypto.so.3 => /usr/lib/libk5crypto.so.3 (0x00007effdff2b000)
	libkrb5support.so.0 => /usr/lib/libkrb5support.so.0 (0x00007effdfd22000)
	libkeyutils.so.1 => /lib/libkeyutils.so.1 (0x00007effdfb20000)
	libresolv.so.2 => /lib/libresolv.so.2 (0x00007effdf90a000)
	liblber-2.4.so.2 => /usr/lib/liblber-2.4.so.2 (0x00007effdf6fb000)
	libsasl2.so.2 => /usr/lib/libsasl2.so.2 (0x00007effdf4e2000)
	libgnutls.so.26 => /usr/lib/libgnutls.so.26 (0x00007effdf23f000)
	libtasn1.so.3 => /usr/lib/libtasn1.so.3 (0x00007effdf02e000)
	libgcrypt.so.11 => /usr/lib/libgcrypt.so.11 (0x00007effdedb6000)
	libgpg-error.so.0 => /usr/lib/libgpg-error.so.0 (0x00007effdebb2000)
$ ls -l /usr/lib/libpq.so.5
lrwxrwxrwx 1 root root 12 Jan 29 18:01 /usr/lib/libpq.so.5 -> libpq.so.5.2

[mcayland]

Right so from my PostgreSQL 9.0 installation:

g90@kentang:~/rel/lib$ ls -l |grep libpq -rw-r--r-- 1 pg90 pg90 779062 Mar 26 12:38 libpq.a lrwxrwxrwx 1 pg90 pg90 12 Mar 26 12:38 libpq.so -> libpq.so.5.3 lrwxrwxrwx 1 pg90 pg90 12 Mar 26 12:38 libpq.so.5 -> libpq.so.5.3 -rwxr-xr-x 1 pg90 pg90 440772 Mar 26 12:38 libpq.so.5.3

Given that your version of libpq is 5.2, I suspect that you're actually linking against a PostgreSQL 8.4 version of libpq instead which is why you see the error. Now the general philosophy of the PostgreSQL project is that newer versions of libraries can always talk to older versions of libraries, but not the other way around. My concern is that we can find ourselves going down a slippery and unsupported route by doing this...

Note that PostGIS 2.1 will no longer support PostgreSQL < 9.0 so this won't be a problem for newer versions moving forward; if you can still come up with a minimally invasive patch that uses version detection then I would consider applying it just to 2.0 branch.

[dfuhry2]

Unfortunately we can't directly condition the change on PQlibVersion() since it's only available since 9.1: ​http://www.postgresql.org/docs/9.1/static/libpq-misc.html Also unfortunately, the PG_VERSION and PG_VERSION_NUM constants match the server version (9.2 in my case), not the libpq version. I guess PQlibVersion() could be tested for in configure, that still wouldn't distinguish between users of 9.0's libpq and 8.4's. Not clear to me how to make that distinction.

if (PQlibVersion() < 90000)
{
    ...
}

[mcayland]

That does seem to be an excessively complicated approach - why not simply just always send "SET bytea_output = 'escape'" if PQserverVersion() returns something >= 9.0 (see ​http://www.postgresql.org/docs/8.4/static/libpq-status.html).

[mcayland]

Actually on second thoughts, that will penalise people using a new version of libpq against a new PostgreSQL server since the point of hex encoding is that it noticeably reduces the traffic on the wire between client and server compared to escape encoding :(

Perhaps a better approach would be to perform PQunescapeBytea() on a hex-encoded string such as '\x4d' - if the client library supports hex encoding you'll get back an 'M', otherwise something different? I haven't tried this, but it might be worth an experiment.

[robe]

changing title -- it has nothing to do with 9.2 and is a libpq mismatch between server and client. I say push. All options sound not nice at this point and I question it affects that many people to begin with.

[mcayland]

Possibly the only sensible way to do this would be to have another command line option to force this setting where required, however because using older libpq is not a generally supported setup on PostgreSQL and we're not going to support 8.4 when 2.1 comes out anyway then I'm tempted to set this to 'wontfix'.

[robe]

+1 - yes let's not fix.

I think the newer libpqs are already smart enough to work with older PostgreSQL. So it's only an issue for those trying to use an older libpq with a newer postgreSQL install. They shouldn't be doing that anyway and we shouldn't be encouraging it. Even PostgreSQL for example doesn't allow you to backup/restore a PostgreSQL database with an older pg_dump/pg_restore. I see this as much the same thing :)