As discussed with Tom in [1] and again with Pavel and Alvaro in [2], here is a partial WIP refactoring of the SPI interface. The goal is to remove as many of the SPI_ERROR_xxx codes as possible from the interface, replacing them with elog(ERROR), without removing the ability of callers to check meaningful return codes and make their own decisions about what to do next. The crucial point here is that many of the error codes in SPI are "can't happen" or "you made a programmatic mistake" type errors that don't require run time remediation, but rather require fixing the C code and recompiling. Those seem better handled as an elog(ERROR) to avoid the need for tests against such error codes sprinkled throughout the system.
One upshot of the refactoring is that some of the SPI functions that previously returned an error code can be changed to return void. Tom suggested a nice way to use macros to provide backward compatibility with older third-party software, and I used his suggestion.
I need guidance with SPI_ERROR_ARGUMENT because it is used by functions that don't return an integer error code, but rather return a SPIPlanPtr, such as SPI_prepare. Those functions return NULL and set a global variable named SPI_result to the error code. I'd be happy to just convert this to throwing an error, too, but it is a greater API break than anything implemented in the attached patches so far. How do others feel about it?
If more places like this can be converted to use elog(ERROR), it may be possible to convert more functions to return void.
Generally lot of API used by extensions are changing - SPI is not different, and I don't see too much benefit of compatibility API. When you need to define BACKWARDS_COMPATIBLE_SPI_CALLS, then you can clean code.
It looks for me needlessly. If we change internal API, then should be clean signal so code should be fixed, so I don't like
-#define SPI_ERROR_PARAM (-7) +#define SPI_ERROR_PARAM (-7) /* not used anymore */
It should be removed.
I am maybe too aggressive - but because any extension should be compiled for any postgres release, I don't think so we should to hold some internal obsolete API. BACKWARDS_COMPATIBLE.. is not used else where, and I would not to introduce this concept here. It can helps in short perspective, but it can be trap in long perspective.
