ProcessUtility_hook
in [PgSql]
|
From: Euler Taveira de Oliveira on 20 Oct 2009 06:58 Itagaki Takahiro escreveu: > We can hook SELECT, INSERT, UPDATE and DELETE with ExecutorXxx_hooks, > but there is no way to hook DDL commands. Can I submit a patch to add > ProcessUtility_hook? > +1. Hey, I was thinking about that yesterday. ;) > pg_stat_statements module also handle DDL commands as same as normal > queries. Fortunately, autovacuum calls call vacuum() directly, > so the statistics will not filled with VACUUM and ANALYZE commands > by autovacuum. > I don't look at the code but maybe there is a way to hook something there too. But I'd leave it alone for now, unless it's few lines of code. -- Euler Taveira de Oliveira http://www.timbira.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Itagaki Takahiro on 20 Oct 2009 21:14 Here is a patch to add ProcessUtility_hook to handle all DDL commands in user modules. (ProcessUtility_hook_20091021.patch) It adds a global variable 'ProcessUtility_hook' and export the original function as 'standard_ProcessUtility'. Euler Taveira de Oliveira <euler(a)timbira.com> wrote: > > pg_stat_statements module also handle DDL commands as same as normal > > queries. Fortunately, autovacuum calls call vacuum() directly, > > so the statistics will not filled with VACUUM and ANALYZE commands > > by autovacuum. > > > I don't look at the code but maybe there is a way to hook something there too. > But I'd leave it alone for now, unless it's few lines of code. I see it is debatable whether pg_stat_statements should support the hook. So I split the change in another patch. (pgss_utility_hook_20091021.patch) Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
From: Euler Taveira de Oliveira on 18 Nov 2009 22:31 Itagaki Takahiro escreveu: > Here is a patch to add ProcessUtility_hook to handle all DDL > commands in user modules. (ProcessUtility_hook_20091021.patch) > It adds a global variable 'ProcessUtility_hook' and > export the original function as 'standard_ProcessUtility'. > I've reviewed your patch. It applies cleanly and compiles without warnings. It lacks documentation. Could you update it? The functionality is divided in two parts. The first part is a hook in the utility module. The idea is capture the commands that doesn't pass through executor. I'm afraid that that hook will be used only for capturing non-DML queries. If so, why don't we hack the tcop/postgres.c and grab those queries from the same point we log statements? This feature is similar to the executor one. But in the executor case, we use the plan for other things. Other utilities are (i) using that hook to filter out some non-DML commands and (ii) developing some non-DML replication mechanism for trigger-based replication solutions. The second part is to use that hook to capture non-DML commands for pg_stat_statements module. Do we need to have rows = 0 for non-DML commands? Maybe NULL could be an appropriate value. The PREPARE command stopped to count the number of rows. Should we count the rows in EXECUTE command or in the PREPARE command? The other command that doesn't count properly is COPY. Could you fix that? I'm concerned about storing some commands that expose passwords like CREATE ROLE foo PASSWORD 'secret'; I know that the queries are only showed to superusers but maybe we should add this information to documentation or provide a mechanism to exclude those commands. I don't know if it is worth the trouble adding some code to capture VACUUM and ANALYZE commands called inside autovacuum. -- Euler Taveira de Oliveira http://www.timbira.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Itagaki Takahiro on 30 Nov 2009 02:26 Euler Taveira de Oliveira <euler(a)timbira.com> wrote: > The functionality is divided in two parts. The first part is a hook in the > utility module. The idea is capture the commands that doesn't pass through > executor. I'm afraid that that hook will be used only for capturing non-DML > queries. If so, why don't we hack the tcop/postgres.c and grab those queries > from the same point we log statements? DDLs can be used from user defined functions. It has the same reason why we have executor hooks instead of tcop hooks. > The second part is to use that hook to capture non-DML commands for > pg_stat_statements module. - I fixed a bug that it should handle only isTopLevel command. - A new parameter pg_stat_statements.track_ddl (boolean) is added to enable or disable the feature. > Do we need to have rows = 0 for non-DML commands? > Maybe NULL could be an appropriate value. Yes, but it requires additional management to handle 0 and NULL separately. I don't think it is worth doing. > The PREPARE command stopped to count > the number of rows. Should we count the rows in EXECUTE command or in the > PREPARE command? It requires major rewrites of EXECUTE command to pass the number of affected rows to caller. I doubt it is worth fixing because almost all drivers use protocol-level prepared statements instead of PREPARE+EXECUTE. > The other command that doesn't count properly is COPY. Could > you fix that? I added codes for it. > I'm concerned about storing some commands that expose passwords > like CREATE ROLE foo PASSWORD 'secret'; I know that the queries are only > showed to superusers but maybe we should add this information to documentation > or provide a mechanism to exclude those commands. I think there is no additonal problem there because we can see the 'secret' command using pg_stat_activity even now. > I don't know if it is worth the trouble adding some code to capture VACUUM and > ANALYZE commands called inside autovacuum. I'd like to exclude VACUUM and ANALYZE by autovacuum because pg_stat_statements should not filled with those commands. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
From: Bruce Momjian on 30 Nov 2009 20:11 I have applied this patch, with only a minor wording improvement: Specify <literal>on</> to track DDL commands, which excludes <command>SELECT</>, ^^^^^^^^^^^^^^ Thanks. --------------------------------------------------------------------------- Itagaki Takahiro wrote: > > Euler Taveira de Oliveira <euler(a)timbira.com> wrote: > > > The functionality is divided in two parts. The first part is a hook in the > > utility module. The idea is capture the commands that doesn't pass through > > executor. I'm afraid that that hook will be used only for capturing non-DML > > queries. If so, why don't we hack the tcop/postgres.c and grab those queries > > from the same point we log statements? > > DDLs can be used from user defined functions. It has the same reason > why we have executor hooks instead of tcop hooks. > > > > The second part is to use that hook to capture non-DML commands for > > pg_stat_statements module. > > - I fixed a bug that it should handle only isTopLevel command. > - A new parameter pg_stat_statements.track_ddl (boolean) is > added to enable or disable the feature. > > > Do we need to have rows = 0 for non-DML commands? > > Maybe NULL could be an appropriate value. > > Yes, but it requires additional management to handle 0 and NULL > separately. I don't think it is worth doing. > > > The PREPARE command stopped to count > > the number of rows. Should we count the rows in EXECUTE command or in the > > PREPARE command? > > It requires major rewrites of EXECUTE command to pass the number of > affected rows to caller. I doubt it is worth fixing because almost all > drivers use protocol-level prepared statements instead of PREPARE+EXECUTE. > > > The other command that doesn't count properly is COPY. Could > > you fix that? > > I added codes for it. > > > I'm concerned about storing some commands that expose passwords > > like CREATE ROLE foo PASSWORD 'secret'; I know that the queries are only > > showed to superusers but maybe we should add this information to documentation > > or provide a mechanism to exclude those commands. > > I think there is no additonal problem there because we can see the 'secret' > command using pg_stat_activity even now. > > > > I don't know if it is worth the trouble adding some code to capture VACUUM and > > ANALYZE commands called inside autovacuum. > > I'd like to exclude VACUUM and ANALYZE by autovacuum because > pg_stat_statements should not filled with those commands. > > > Regards, > --- > ITAGAKI Takahiro > NTT Open Source Software Center > [ Attachment, skipping... ] > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <bruce(a)momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
|
Next
|
Last
Pages: 1 2 3 4 5 Prev: UTF8 with BOM support in psql Next: alpha1 bundled -- please verify |