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


Reply via email to