From: | Mark Dilger <hornschnorter(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Checking return value of SPI_execute |
Date: | 2019-11-06 15:35:18 |
Message-ID: | 4bc8eca7-6efa-8174-a8cd-69167f233f6d@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 11/5/19 8:27 PM, Michael Paquier wrote:
> 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.
Other code that checks the return value from an SPI function is
inconsistent about whether it checks for SPI_OK_SELECT or simply checks
for a negative result. I was on the fence about which precedent to
follow, and was just slightly in favor of testing for negative rather
than SPI_OK_SELECT due to this function, query_to_oid_list, taking the
query string as an argument and not controlling whether that argument is
indeed a plain SELECT.
I don't feel strongly about it.
Mark Dilger
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2019-11-06 15:38:05 | Re: TAP tests aren't using the magic words for Windows file access |
Previous Message | Tom Lane | 2019-11-06 15:29:29 | Re: PostgreSQL 12 installation fails because locale name contained non-english characters |