Mark Dilger <hornschnorter@gmail.com> writes:
> On Fri, May 17, 2019 at 6:12 PM Tom Lane <tgl@sss.pgh.pa.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
