Thanks for reviewing this. I will spend some time looking at your recommendations in detail and seeing what it would take to implement them, and whether I agree that is better; but I wanted to point out a couple things regarding the SPI interface where I'm not sure you realize what's currently being done. I may want to argue some of the rest if I don't agree after more detailed review; this is just what jumps out on a first pass.
Heikki Linnakangas <hlinnakan...@vmware.com> wrote: > instead of passing parameters to the SPI calls individually, you > invented SPI_register_tuplestore which affects all subsequent SPI > calls. All subsequent SPI calls on that particular SPI connection until it is closed, except for any tuplestores are later unregistered. Nested SPI connections do not automatically inherit the named tuplestores; whatever code opens an SPI connection would need to register them for the new context, if desired. This seemed to me to provide minimal disruption to the existing SPI callers who might want to use this. > The next question is how to pass the new hooks and tuplestores > through the SPI interface. For prepared plans, the current > SPI_prepare_params + SPI_execute_plan_with_paramlist functions > work fine. They work fine, I guess, in the *one* place they are used. SPI_prepare_params() is called exactly *once* from plpgsql's pl_exec.c, and SPI_execute_plan_with_paramlist() is called twice from the same file. There are no other calls to either from anywhere else in the code base. > However, there doesn't seem to be any way to do one-shot queries > with a ParserSetupHook and ParamListInfo. That seems like an > oversight, and would be nice to address that anyway. There are dozens of SPI_prepare* and SPI_exec* calls scattered all over the backend, pl, and contrib code which appear to get along fine without that. Partly it may be because it involves something of a modularity violation; the comment for the function used for this call (where it *is* used) says: /* * plpgsql_parser_setup set up parser hooks for dynamic parameters * * Note: this routine, and the hook functions it prepares for, are logically * part of plpgsql parsing. But they actually run during function execution, * when we are ready to evaluate a SQL query or expression that has not * previously been parsed and planned. */ Can you clarify what benefit you see to modifying the SPI API the way you suggest, and what impact it might have on existing calling code? > PS. the copy/read/write functions for TuplestoreRelation in the > patch are broken; TuplestoreRelation is not a subclass of Plan. I'm not sure what you mean by "broken" -- could you elaborate? It is, in a lot of ways, parallel to the CommonTableExpr defined a little above it in the parsenodes.h file -- a relation which can only be referenced by unqualified name. It is after CTEs in search order, and before anything else. Having such a node in the tree after parse analysis allows a number of special cases to be handled pretty much the same as they are for CTEs, which seems like a good thing to me. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
