st 6. 11. 2019 v 16:38 odesÃlatel Mark Dilger <hornschnor...@gmail.com> napsal:
> > > On 11/5/19 9:54 PM, Pavel Stehule wrote: > > > > > > st 6. 11. 2019 v 5:28 odesÃlatel Michael Paquier <mich...@paquier.xyz > > <mailto:mich...@paquier.xyz>> napsal: > > > > On Tue, Nov 05, 2019 at 05:21:25PM -0800, Mark Dilger wrote: > > > please find attached a patch fixing a problem previously > > discussed [1] about > > > the code inappropriately ignoring the return value from > SPI_execute. > > > > > > I will be adding this to https://commitfest.postgresql.org/26/ > > > shortly. > > > > Yes, this should be fixed. > > > > > - SPI_execute(query, true, 0); > > > + spi_result = SPI_execute(query, true, 0); > > > + if (spi_result < 0) > > > + elog(ERROR, "SPI_execute returned %s", > > SPI_result_code_string(spi_result)); > > > > Any queries processed in xml.c are plain SELECT queries, so it seems > > to me that you need to check after SPI_OK_SELECT as only valid > > result. > > > > > > 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 creating another patch which removes most of the error codes from > the interface and uses elog(ERROR) or ereport(ERROR) instead, but I > anticipate a lot of debate about that design and wanted to get this > simpler patch into the queue. I don't think we need to reject this > patch in favor of redesigning the entire SPI API. Instead, we can apply > this patch as a simple bug fix, and then if it gets removed later when > the other, larger patch is committed, so be it. > > Does that plan seem acceptable? > I am not against these fix. Regards Pavel > > Mark Dilger >
