Re: Checking return value of SPI_execute

Поиск
Список
Период
Сортировка
От Pavel Stehule
Тема Re: Checking return value of SPI_execute
Дата
Msg-id CAFj8pRAuzJNOCm3SCu2oZBMKKN0A8KaRhk+Sn2jUeYo_LDpE_g@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Checking return value of SPI_execute  (Michael Paquier <michael@paquier.xyz>)
Ответы Re: Checking return value of SPI_execute  (Michael Paquier <michael@paquier.xyz>)
Re: Checking return value of SPI_execute  (Mark Dilger <hornschnorter@gmail.com>)
Список pgsql-hackers


st 6. 11. 2019 v 5:28 odesílatel Michael Paquier <michael@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?

Pavel



--
Michael

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tatsuro Yamada
Дата:
Сообщение: Re: progress report for ANALYZE
Следующее
От: vignesh C
Дата:
Сообщение: Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions