Re: Additional SPI functions

In the event that my plpython3 patch does not make it, it seems prudent to try and get a *much* smaller patch in to allow the PL to easily exist out of core.

I added a couple SPI functions in order to support the database access functionality in plpython3u. Also, a getelevel() function for conditionally including context information due to error trapping awkwardness:

extern int SPI_execute_statements(const char *src);

Execute multiple statements. Intended, primarily, for executing one or more DDL or DML statements. In contrast with the other execution functions, the RPT loop plans and executes the statement before planning and executing the next in order to allow subsequent statements to see the effects of all the formers. The read only argument is "omitted" as it should only be used in read-write cases(you can't read anything out of it).

extern SPIPlanPtr SPI_prepare_statement(
const char *src, int cursorOptions,
SPIParamCallback pcb, void *pcb_arg,
TupleDesc *resultDesc);

Prepare a *single* statement and call the SPIParamCallback with the parameter information allowing the caller to store the information and supply constant parameters based on the identified parameter types, if need be. Also, if it returns rows, return the TupleDesc via *resultDesc.

typedef void (*SPIParamCallback)(
void *cb_data, const char *commandTag,
int nargs, Oid *typoids, Datum **param_values, char **param_nulls);

Not at all in love with the callback, but it seemed desirable over using an intermediate structure that would require some additional management.

Certainly, docs and tests will be necessary for this, but I'm sending it out now with the hopes of getting some feedback before sweating those tasks.

The patch is attached for easy reference.
Any help would, of course, be greatly appreciated.

cheers

James William Pye <lists(at)jwp(dot)name> writes:
> extern int SPI_execute_statements(const char *src);

> Execute multiple statements. Intended, primarily, for executing one or more DDL or DML statements. In contrast with the other execution functions, the RPT loop plans and executes the statement before planning and executing the next in order to allow subsequent statements to see the effects of all the formers. The read only argument is "omitted" as it should only be used in read-write cases(you can't read anything out of it).

This seems just about entirely useless. Why not code a loop around one
of the existing SPI execution functions?

> extern SPIPlanPtr SPI_prepare_statement(
> const char *src, int cursorOptions,
> SPIParamCallback pcb, void *pcb_arg,
> TupleDesc *resultDesc);

> Prepare a *single* statement and call the SPIParamCallback with the parameter information allowing the caller to store the information and supply constant parameters based on the identified parameter types, if need be. Also, if it returns rows, return the TupleDesc via *resultDesc.

This looks like it's most likely redundant with the stuff I added
recently for the plpgsql parser rewrite. Please see if you can use that
instead.

regards, tom lane

On Dec 20, 2009, at 12:03 AM, Tom Lane wrote:
> Why not code a loop around one of the existing SPI execution functions?

*shrug* seemed nicer to push it on the parser than to force the user to split up the statements/calls. Or split up the statements myself(well, the parser does it so swimmingly =).

It's purpose is to allow the user to put a chunk of SQL into a single big block:

sqlexec("""
CREATE TEMP TABLE one ...;
CREATE TEMP TABLE two ...;
<init temp tables with data for use in the procedure>
""")

For me, that tends to read better than breaking up the calls.
Well, the above may be a bad example for crying about readability, but I'm thinking of cases with a bit more SQL in em'..

[spi_prepare_statement]
> This looks like it's most likely redundant with the stuff I added
> recently for the plpgsql parser rewrite.

If that allows me to identify the parameter type Oids of the statement, optionally supply constant parameters after identifying the types(so I can properly create the parameter Datums), and provides access to the resultDesc, then yes it is redundant. Personally, I'm hoping for redundant. =)

> Please see if you can use that instead.

I took a very short peak (wasn't really looking..) earlier today (err yesterday now) and nothing jumped out at me, but I'll take a closer look now.

Thanks =)

On Dec 20, 2009, at 12:03 AM, Tom Lane wrote:
> This looks like it's most likely redundant with the stuff I added
> recently for the plpgsql parser rewrite. Please see if you can use that
> instead.

The parser param hooks will definitely work. As for getting the result TupleDesc prior to execution, I can include spi_priv.h and look at the CPS list directly. Something more crayola would be preferable, but I don't think "SPI_prepare_statement" is that something; although, it did make for a fine stopgap. (Well, "fine", saving that my proposed SPI_prepare_statement appeared to be broken wrt plan revalidation and bound parameters.. ew)

So, after looking into the parser hooks, CachedPlanSource, and SPI more, I ended up taking a slightly different route. I expect it to work with a couple prior versions of PG as well, so there is some added value over a new SPI function or exclusively using param hooks. And, now, thinking of compatibility with past versions of PG, I'll find a different route for "SPI_execute_statements" as well.

Thanks.