Re: Checking return value of SPI_execute

On Wed, Nov 06, 2019 at 06:54:16AM +0100, Pavel Stehule wrote:
> Is generic question if this exception should not be raised somewhere in
> spi.c - maybe at SPI_execute.
>
> When you look to SPI_execute_plan, then checked errors has a character +/-
> assertions. All SQL errors are ended by a exception. This API is not too
> consistent after years what is used.
>
> I agree so this result code should be tested for better code quality. But
> this API is not consistent now, and should be refactored to use a
> exceptions instead result codes. Or instead error checking, a assertions
> should be used.
>
> What do you think about it?

I am not sure what you are proposing here, nor am I sure to what kind
of assertions you are referring to in spi.c. If we were to change the
error reporting, what of the external and existing consumers of this
routine? They would not expect to bump on an exception and perhaps
need to handle error code paths by themselves, no?

Anyway, any callers of SPI_execute() (tablefunc.c, matview.c) we have
now in-core react based on a status or a set of statuses they expect,
so based on that fixing this caller in xml.c sounds fine to me.
--
Michael