Re: Is it safe to ignore the return value of SPI_finish and SPI_execute?

Mark Dilger <hornschnorter(at)gmail(dot)com> writes:
> On Fri, May 17, 2019 at 6:12 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> One reasonable solution would be to change the callers that got this
>> wrong. Another one would be to reconsider whether the error-return-code
>> convention makes any sense at all here. If we changed the above-quoted
>> bit to be an ereport(ERROR), then we could say that SPI_finish either
>> returns 0 or throws error, making it moot whether callers check, and
>> allowing removal of now-useless checks from all the in-core callers.

> Does this proposal of yours seem good enough for me to make a patch
> based on this design?

Just to clarify --- I think what's being discussed here is "change some
large fraction of the SPI functions that can return SPI_ERROR_xxx error
codes to throw elog/ereport(ERROR) instead". Figuring out what fraction
that should be is part of the work --- but just in a quick scan through
spi.c, it seems like there might be a case for deprecating practically
all the SPI_ERROR_xxx codes except for SPI_ERROR_NOATTRIBUTE.
I'd definitely argue that SPI_ERROR_UNCONNECTED and SPI_ERROR_ARGUMENT
deserve that treatment.

I'm for it, if you want to do the work, but I don't speak for everybody.

It's not entirely clear to me whether we ought to change the return
convention to be "returns void" or make it "always returns SPI_OK"
for those functions where the return code becomes trivial. The
latter would avoid churn for external modules, but it seems not to
have much other attractiveness.

regards, tom lane