|From:||Jan Urbański <wulczer(at)wulczer(dot)org>|
|To:||Steve Singer <ssinger_pg(at)sympatico(dot)ca>|
|Cc:||Peter Eisentraut <peter_e(at)gmx(dot)net>, Postgres - Hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: pl/python custom exceptions for SPI|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 10/02/11 22:26, Steve Singer wrote:
> On 11-02-10 03:13 PM, Jan Urbański wrote:
>> On 10/02/11 20:24, Peter Eisentraut wrote:
> Here is the rest of my review.
> Ideally char * members of ExceptionMap would be const, but since many
> versions of python take a non-const value to PyErr_NewException that
> won't work :(
Yeah, I got "discards qualifier" warnings when I tried to declare it as
> After you search the for an exception in the hash you have:
> /* We really should find it, but just in case have a fallback */
> Assert(entry != NULL);
> exc = entry ? entry->exc : PLy_exc_spi_error;
> I'm not sure the assert is needed here. Just falling back to the
> exception type seems reasonable and more desirable than an assert if
> showhow a new exception gets missed from the list. I don't feel that
> strongly on this.
Maybe the comment doesn't explain this enough. My intention was that in
regular builds you have a fallback and if you're missing an entry in the
exceptions hash, you just get SPIError. But in assert-enabled builds you
get an error, so you can detect it and fix the exceptions hash.
> line 3575: PLy_elog(ERROR, "Failed to add the spiexceptions module");
> "Failed" should be "failed"
Gah, I'll never learn. Will fix.
> Other than that the patch looks fine to me.
Thanks, I'll have the docs ready by today (and I should've have them by
|Next Messageemail@example.com||2011-02-11 10:03:37||Re: Sorting. When?|
|Previous Message||Dimitri Fontaine||2011-02-11 09:44:32||Re: ALTER EXTENSION UPGRADE, v3|