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 my pg_stat_statements plugin, the tag is used to cache hash values of > SQL strings in PlannedStmt. It is not necessarily needed because the hash > value is re-computable from debug_query_string. It is just for avoiding > the work. In addition, we see different SQLs in debug_query_string in > PREPARE/EXECUTE and DECLARE/FETCH. Hashed SQL cache can work on those > commands. Actually, that aspect of the plugin is 100% broken anyway, because it assumes that debug_query_string has got something to do with the query being executed. There are any number of scenarios where this is a bad assumption. 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 space and copying. It would amount to a permanent decision that we're willing to pay the overhead of keeping the source text around, though. Also, after looking at the patch more closely, was there a good reason for making the hook intercept ExecutePlan rather than ExecutorRun? ExecutePlan was never intended to have a stable public API --- its argument list is just a happenstance of what ExecutorRun needs to fetch for its own purposes. I think we should keep it private and have ExecutorRun do if (hook) hook(...); else standard_ExecutorRun(...); regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers