Re: Add CREATE support to event triggers
От | Michael Paquier |
---|---|
Тема | Re: Add CREATE support to event triggers |
Дата | |
Msg-id | CAB7nPqQJBYmS6wvtTcTSHzy7T03tLfCTY-AePpkaHnfc_rhsxg@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Add CREATE support to event triggers (Alvaro Herrera <alvherre@2ndquadrant.com>) |
Ответы |
Re: Add CREATE support to event triggers
(Michael Paquier <michael.paquier@gmail.com>)
|
Список | pgsql-hackers |
On Mon, Oct 13, 2014 at 12:45 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Here's a new version of this series. The main change is that I've > changed deparse_utility.c to generate JSON, and the code that was in > commands/event_trigger.c to decode that JSON, so that it uses the new > Jsonb API instead. In addition, I've moved the new code that was in > commands/event_trigger.c to utils/adt/ddl_json.c. (The only entry point > of the new file is the SQL-callable pg_event_trigger_expand_command() > function, and its purpose is to expand a JSON object emitted by the > deparse_utility.c code back into a plain text SQL command.) > I have also cleaned up the code per comments from Michael Paquier and > Andres Freund: > > * the GRANT support for event triggers now correctly ignores global > objects. > > * COMMENT ON .. IS NULL no longer causes a crash Why would this not fail? Patch 3 in this set is identical to the last one. I tested that and it worked well... > * renameatt() and ExecRenameStmt are consistent in returning the > objSubId as an "int" (not int32). This is what is used as objectSubId > in ObjectAddress, which is what we're using this value for. In patch 1, ExecRenameStmt returns objsubid for an attribute name when rename is done on an attribute. Now could you clarify why we skip this list of objects even if their sub-object ID is available with address.objectSubId? case OBJECT_AGGREGATE: case OBJECT_COLLATION: case OBJECT_CONVERSION: case OBJECT_EVENT_TRIGGER: case OBJECT_FDW: case OBJECT_FOREIGN_SERVER: case OBJECT_FUNCTION: case OBJECT_OPCLASS: case OBJECT_OPFAMILY: case OBJECT_LANGUAGE: case OBJECT_TSCONFIGURATION: case OBJECT_TSDICTIONARY: case OBJECT_TSPARSER: case OBJECT_TSTEMPLATE: Patch 2 fails on make check for the tests privileges and foreign_data by showing up double amount warnings for some object types: *** /Users/ioltas/git/postgres/src/test/regress/expected/privileges.out Fri Oct 10 14:34:10 2014 --- /Users/ioltas/git/postgres/src/test/regress/results/privileges.outThu Oct 16 15:47:42 2014 *************** *** 118,123 **** --- 118,124 ---- ERROR: permission denied for relation atest2 GRANT ALL ON atest1 TO PUBLIC; -- fail WARNING: no privilegeswere granted for "atest1" + WARNING: no privileges were granted for "atest1" -- checks in subquery, both ok SELECT * FROM atest1 WHERE ( b IN ( SELECTcol1 FROM atest2 ) ); a | b EventTriggerSupportsGrantObjectType is fine to remove ACL_OBJECT_DATABASE and ACL_OBJECT_TABLESPACE from the list of supported objects. That's as well in line with the current firing matrix. I think that it would be appropriate to add a comment on top of this function. Patch 3 looks good. Regards, -- Michael
В списке pgsql-hackers по дате отправления: