|From:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|To:||Mark Dilger <hornschnorter(at)gmail(dot)com>|
|Subject:||Re: Is it safe to ignore the return value of SPI_finish and SPI_execute?|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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
|Next Message||Finnerty, Jim||2019-05-22 21:03:08||Re: Why could GEQO produce plans with lower costs than the standard_join_search?|
|Previous Message||Tom Lane||2019-05-22 20:13:02||Re: FullTransactionId changes are causing portability issues|