On Sat, Oct 05, 2019 at 04:48:40PM +0200, Christoph Zwerschke wrote:
> >Find attached minimal patch.
>
> Thank you, looks fine. I've already added docs and tests as well, but not
> yet committed to trunk.
I didn't mean you had to stop and do that - I hammered it out while waiting for
pg_checksums to run :)
Also, it occurred to me this "else" is unnecessary:
+#if PG_VERSION_NUM >= 120000 /* Compile time version */
+ if (PQlibVersion() >= 120000) /* Runtime version */
+ return PyLong_FromSize_t(PQresultMemorySize(self->result));
+ else
> In addition to query.memsize() I've also added get_pqlib_version() as a module
> level function.
Good idea. But I wonder whether it should be a property, which you suggested
before for types of queryResult columns (for which I sent a WIP patches July 19
and 20).
> But I wonder if we should create a new minor version (5.2) instead of patch
> release (5.1.1) since it's new functionality?
It seems to me the usual procedure is to either issue a security release with
no other changes at all (not for py37, not for pg12, etc). Or include the
security update with the next release. FWIW, your schema-qualification commit
is already out there for anyone who wants it, and the CVE is from last year.
But, that's not an important patch; I don't think we'll use it at telsasoft
(preferring iterators); I think 5.2 should have more than a trivial new
feature.
> Another thing I'd also like to address is the overall handling of
> functionality that is optional or does only work with newer pqlib versions.
> Currently these are behind compiler switches which are set in setup.py, by
> default they are enabled if the pqlib version is new enough to support the
> respective functionality.
I think you're referring to:
$ grep -h '#if' *c |sort |uniq -c |sort -nr
... LARGE_OBJECTS DEFAULT_VARS ESCAPING_FUNCS DIRECT_ACCESS SSL_INFO NO_PQSOCKET
> Alternatively, the pqlib version could be checked directly using pg_config.h
> in the code instead of using the various switches,
> like you are doing here. I'd like to handle this more consistently through
> the code. But this would also warrant a new 5.2 version:
> see https://github.com/Cito/PyGreSQL/issues/12
I thought it ought to have compile time check (for headers and link library
version) and runtime check (for runtime lib version), but, now, even *with* my
runtime library check, this happens if pygres is run against an earlier version
of libpq:
|$ PYTHONPATH=build/lib.linux-x86_64-2.7/ python2.7 -c "import pg; print
pg.DB('postgres').query('SELECT generate_series(1,9999)').memsize()"
|Traceback (most recent call last):
| File "<string>", line 1, in <module>
| File "pg.py", line 27, in <module>
| from _pg import *
|ImportError: /home/pryzbyj/src/pygres/build/lib.linux-x86_64-2.7/_pg.so:
undefined symbol: PQresultMemorySize
..so maybe my runtime check is useless, or maybe libpq isn't doing proper
library versioning? I've forgotten whatever I once knew about this.
$ ldd build/lib.linux-x86_64-2.7/_pg.so |grep libpq
libpq.so.5 => /usr/lib/x86_64-linux-gnu/libpq.so.5 (0x00007fb3f652a000)
Justin
_______________________________________________
PyGreSQL mailing list
[email protected]
https://mail.vex.net/mailman/listinfo/pygresql
