Re: pl/python custom exceptions for SPI

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
Date: 2011-02-11 09:53:01
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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
const :(

> 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
yesterday :/).


In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message 2011-02-11 10:03:37 Re: Sorting. When?
Previous Message Dimitri Fontaine 2011-02-11 09:44:32 Re: ALTER EXTENSION UPGRADE, v3