1. in PLy_spi_error__init__ you need to check kw for NULL before doing PyDict_Size(kw) otherwise for plpy.SPIError() you get Bad internal call because PyDict_Size expects a real dictionary not NULL
PyDict_Size returns -1 when the dictionary is NULL
2. a test with just plpy.SPIError() is still missing, this would have caught 1.
please, can you write some example - I am not able raise described error
3. the tests have "possibility to set all accessable fields in custom exception" above a test that doesn't set all fields, it's confusing (and accessible is spelled wrong)
done
4. in the docs, "The constructor of SPIError exception (class) supports keyword parameters." sounds better as "An SPIError instance can be constructed using keyword parameters."
done
5. there is conceptual code duplication between PLy_spi_exception_set in plpy_spi.c, since that code also constructs an SPIError but from C and with more info available (edata->internalquery and edata->internalpos). But making a tuple and assigning it to spidata is in both. Not sure how this can be improved.
I see it, but I don't think, so current code should be changed. PLy_spi_exception_set is controlled directly by fully filled ErrorData structure, __init__ is based only on keyword parameters with possibility use inherited data. If I'll build ErrorData in __init__ function and call PLy_spi_exception_set, then the code will be longer and more complex.
6. __init__ can use METH_VARARGS | METH_KEYWORDS in both Python2 and 3, no need for the #if
done
7. "could not create dictionary for SPI exception" would be more precise as "could not create dictionary for SPIError" right?
done
8. in PLy_add_methods_to_dictionary I would avoid import since it makes one think of importing modules; maybe "could not create functionwrapper for \"%s\"", "could not create method wrapper for \"%s\""
done
9. also in PLy_add_methods_to_dictionary "could public method \"%s\" in dictionary" is not proper English and is missing not, maybe "could not add method \"%s\" to class dictionary"?
done
10. in PLy_add_methods_to_dictionary if PyCFunction_New succeeds but PyMethod_New fails, func will leak
done
11. it would be nice to have a test for the invalid SQLSTATE code part
done
12. similar to 10, in PLy_spi_error__init__ taking the "invalid SQLSTATE" branch leaks exc_args, in general early returns via PLy_elog will leave the intermediate Python objects leaking
