Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-18 Thread Tom Lane
ITAGAKI Takahiro [EMAIL PROTECTED] writes: The attached patch is the proposal. It adds two global symbols: * ExecutorRun_hook - replacing behavior of ExecutorRun() * standard_ExecutorRun() - default behavior of ExecutorRun() Applied. And also modifies one funtion: * ExecuteQuery() -

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-15 Thread ITAGAKI Takahiro
Simon Riggs [EMAIL PROTECTED] wrote: I wonder whether we ought to change things so that the real query source text is available at the executor level. Since we are (at least usually) storing the query text in cached plans, I think this might just require some API refactoring, not extra

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-15 Thread Simon Riggs
On Tue, 2008-07-15 at 16:25 +0900, ITAGAKI Takahiro wrote: Also, after looking at the patch more closely, was there a good reason for making the hook intercept ExecutePlan rather than ExecutorRun? That raises the question of whether we should have ExecutorStart() and ExecutorEnd()

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-15 Thread Tom Lane
ITAGAKI Takahiro [EMAIL PROTECTED] writes: Simon Riggs [EMAIL PROTECTED] wrote: Also, after looking at the patch more closely, was there a good reason for making the hook intercept ExecutePlan rather than ExecutorRun? That raises the question of whether we should have ExecutorStart() and

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-15 Thread ITAGAKI Takahiro
Tom Lane [EMAIL PROTECTED] wrote: That raises the question of whether we should have ExecutorStart() and ExecutorEnd() hooks as well, to round things off. Yeah, and also ExecutorRewind() hook. I'm happy to put in hooks that there's a demonstrated need for, Hmm, ok. I just want to hook

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-14 Thread Simon Riggs
On Thu, 2008-07-10 at 16:11 -0400, Tom Lane wrote: ITAGAKI Takahiro [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] wrote: I don't want the tag there at all, much less converted to a pointer. What would the semantics be of copying the node, and why? Please justify why you must

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-10 Thread Tom Lane
ITAGAKI Takahiro [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] wrote: I don't want the tag there at all, much less converted to a pointer. What would the semantics be of copying the node, and why? Please justify why you must have this and can't do what you want some other way. In

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-08 Thread Simon Riggs
On Mon, 2008-07-07 at 10:51 -0400, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: On Mon, 2008-07-07 at 11:03 +0900, ITAGAKI Takahiro wrote: One issue is tag field. The type is now uint32. It's enough in my plugin, but if some people need to add more complex structures in

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-07 Thread Simon Riggs
On Mon, 2008-07-07 at 11:03 +0900, ITAGAKI Takahiro wrote: Simon Riggs [EMAIL PROTECTED] wrote: The attached patch (executor_hook.patch) modifies HEAD as follows. - Add tag field (uint32) into PlannedStmt. - Add executor_hook to replace ExecutePlan(). - Move ExecutePlan() to a

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-07 Thread Tom Lane
Simon Riggs [EMAIL PROTECTED] writes: On Mon, 2008-07-07 at 11:03 +0900, ITAGAKI Takahiro wrote: One issue is tag field. The type is now uint32. It's enough in my plugin, but if some people need to add more complex structures in PlannedStmt, Node type would be better rather than uint32. Which

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-07 Thread ITAGAKI Takahiro
Tom Lane [EMAIL PROTECTED] wrote: I don't want the tag there at all, much less converted to a pointer. What would the semantics be of copying the node, and why? Please justify why you must have this and can't do what you want some other way. In my pg_stat_statements plugin, the tag is

Re: [HACKERS] [PATCHES] WIP: executor_hook for pg_stat_statements

2008-07-06 Thread ITAGAKI Takahiro
Simon Riggs [EMAIL PROTECTED] wrote: The attached patch (executor_hook.patch) modifies HEAD as follows. - Add tag field (uint32) into PlannedStmt. - Add executor_hook to replace ExecutePlan(). - Move ExecutePlan() to a global function. The executor_hook.patch is fairly trivial and