On 4/9/15 12:14 PM, Alvaro Herrera wrote: > Alvaro Herrera wrote: >> Executive summary: >> >> There is now a CommandDeparse_hook; >> deparse_utility_command is provided as an extension, intended for 9.6; >> rest of patch would be pushed to 9.5. > > Actually here's another approach I like better: use a new pseudotype, > pg_ddl_command, which internally is just a pointer to a CollectedCommand > struct. We cannot give access to the pointer directly in SQL, so much > like type internal or pg_node_tree the in/out functions should just > error out. But the type can be returned as a column in > pg_event_trigger_ddl_command. An extension can create a function that > receives that type as argument, and return a different representation of > the command; the third patch creates a function ddl_deparse_to_json() > which does that.
I've tested this approach and it returns the same results as the previous approach for all calls to pg_event_trigger_ddl_command() made by the pg_audit regression tests. For my testing I was generally only applying patch 0001 and 0002 (although 0001 on it's own also worked for my case). I applied patch 0003 only for the purpose of being sure it did not break anything, and did not specifically test the functionality of 0002 or 0003. However, I've tested 0001 over a very wide range of commands and have not found any anomalous behaviors. I've reviewed the 0001 and 0002 patches and don't have anything to add beyond what has been mentioned in previous reviews. You've applied your pattern consistently across a wide variety of utility commands which is something of a feat. Since 0003 will not be committed to core, I was more interested in the mechanism used to connect 0003 to 0001 and 0002. I like the new approach better than the old one in that there is no connecting hook now. As you say, it provides more flexibility in terms of using the command data in as many ways as you like. Even better, the user has access to three different representations and can choose which one is best for them. And, of course, they can write code for their own representation using pg_ddl_command, JSON, or SQL. I also like the way the patch has been broken up. The core functionality will finally make event triggers truly useful in a high-level language like pl/pgsql. I can see many uses for them now that pg_event_trigger_ddl_command() is present. From my review and testing, I see no barriers to committing patches 0001 and 0002. I'd love to see this functionality in 9.5. Regards, -- - David Steele da...@pgmasters.net
signature.asc
Description: OpenPGP digital signature
