On Tue, Oct 27, 2015 at 9:34 AM, Pavel Stehule <pavel.steh...@gmail.com> wrote: > Hi > > 2015-10-23 7:34 GMT+02:00 Catalin Iacob <iacobcata...@gmail.com>: >> The current code doesn't build on Python3 because the 3rd argument of >> PyMethod_New, the troubled one you need set to NULL has been removed. >> This has to do with the distinction between bound and unbound methods >> which is gone in Python3. > > > this part of is well isolated, and we can do switch for Python2 and Python3 > without significant problems.
I had a quick look at this and at least 2 things are needed for Python3: * using PyInstanceMethod_New instead of PyMethod_New; as http://bugs.python.org/issue1587 and https://docs.python.org/3/c-api/method.html explain that's the new way of binding a PyCFunction to a class, PyMethod_New(func, NULL) fails * in the PLy_spi_error_methods definition, __init__ has to take METH_VARARGS | METH_KEYWORDS not just METH_KEYWORDS; in Python2 METH_KEYWORDS implied METH_VARARGS so it's not needed (it also doesn't hurt) but if I don't add it, in Python3 I get: ERROR: SystemError: Bad call flags in PyCFunction_Call. METH_OLDARGS is no longer supported! >> Still, the above link shows a (more verbose but maybe better) >> alternative: don't use PyErr_NewException and instead define an >> SPIError type with each slot spelled out explicitly. This will remove >> the "is it safe to set the third argument to NULL" question. > > Should be there some problems, if we lost dependency on basic exception > class? Some compatibility issues? Or is possible to create full type with > inheritance support? It's possible to give it a base type, see at https://hg.python.org/cpython/rev/429cadbc5b10/ this line before calling PyType_Ready: PyComError_Type.tp_base = (PyTypeObject*)PyExc_Exception; PyErr_NewException is a shortcut for defining simple Exception deriving types, usually one defines a type with the full PyTypeObject definition. In the meantime, I had a deeper look at the 2.7.10 code and I trust that PyMethod_New with the last argument set to NULL is ok. Setting that to NULL will lead to the PyMethod representing __init__ im_class being NULL. But that PyMethod object is not held onto by C code, it's added to the SPIError class' dict. From there, it is always retrieved from Python via an instance or via the class (so SPIError().__init__ or SPIError.__init__) which will lead to instancemethod_descr_get being called because it's the tp_descr_get slot of PyMethod_Type and that code knows the class you're retrieving the attribute from (SPIError in this case), checks if the PyMethod already has a not NULL im_class (which SPIError.__init__ doesn't) and, if not, calls PyMethod_New again and passes the class (SPIError) as the 3rd argument. Given this, I think it's ok to keep using PyErr_NewException rather than spelling out the type explicitly. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
