(2010/11/12 19:34), KaiGai Kohei wrote:
> I revised my patch according to the prior suggestions.
> 
I'm sorry. I revised my patch, but not attached.

Please see this attached one.

Thanks,

> Invocation of the hooks is encapsulated within macro, not function:
> 
>    + #define InvokeObjectAccessHook0(access,classId,objectId,subId)        \
>    +   do {                                                                \
>    +       if (object_access_hook)                                         \
>    +           (*object_access_hook)((access),(classId),(objectId),(subId)); \
>    +   } while(0)
> 
> The "0" of tail means that it does not takes any arguments except for
> object-ids, like syscache code.
> It will reduce impact when we want to add arguments of the hooks.
> 
> 
> In the previous version, it just support seven object classes that is
> allowed to assign security labels. But, Robert pointed out the purpose
> of post-creation hook is limited to security labeling. So, I expand its
> coverage into all the commentable object classes.
> 
>   - relations:        heap_create_with_catalog()
>   - constraint:       CreateConstraintEntry()
>   - conversion:       ConversionCreate()
>   - schema:   NamespaceCreate()
>   - operator: OperatorCreate() and OperatorShellMake()
>   - procedure:        ProcedureCreate()
>   - type:     TypeCreate() and TypeShellMake()
>   - database: createdb()
>   - cast:     CreateCast()
>   - opfamily: CreateOpFamily()
>   - opclass:  DefineOpClass()
>   - language: create_proc_lang()
>   - attribute:        ATExecAddColumn()
>   - tablespace:       CreateTableSpace()
>   - trigger:  CreateTrigger()
>   - ts_parser:        DefineTSParser()
>   - ts_dict:  DefineTSDictionary()
>   - ts_template:      DefineTSTemplate()
>   - ts_config:        DefineTSConfiguration()
>   - role:     CreateRole()
>   - rule:     InsertRule()
>   - largeobject:      inv_create()
> 
> The post-creation hooks are put on the place just after adding dependency
> of the new object, if the object class uses dependency mechanism.
> I believe it will be a clear guidance for the future maintenance works.
> 
> Thanks,
> 
> (2010/11/11 7:41), KaiGai Kohei wrote:
>> (2010/11/11 3:00), Robert Haas wrote:
>>> On Wed, Nov 10, 2010 at 8:33 AM, KaiGai Kohei<kai...@kaigai.gr.jp>  wrote:
>>>> (2010/11/10 13:06), Robert Haas wrote:
>>>>>>
>>>>>> In this patch, we put InvokeObjectAccessHook0 on the following functions.
>>>>>>
>>>>>> - heap_create_with_catalog() for relations/attributes
>>>>>> - ATExecAddColumn() for attributes
>>>>>> - NamespaceCreate() for schemas
>>>>>> - ProcedureCreate() for aggregates/functions
>>>>>> - TypeCreate() and TypeShellMake() for types
>>>>>> - create_proc_lang() for procedural languages
>>>>>> - inv_create() for large objects
>>>>>
>>>>> I think you ought to try to arrange to avoid the overhead of a
>>>>> function call in the common case where nobody's using the hook.
>>>>> That's why I originally suggested making InvokeObjectAccessHook() a
>>>>> macro around the actual function call.
>>>>>
>>>> Hmm. Although I have little preference here, the penalty to call
>>>> an empty function (when no plugins are installed) is not visible,
>>>> because frequency of DDL commands are not high.
>>>> Even so, is it necessary to replace them by macros?
>>>
>>> It's a fair point. I'm open to other opinions but my vote is to shove
>>> a macro in there. A pointer test is cheaper than a function call, and
>>> doesn't really complicate things much.
>>>
>> Since I have no strong preference function call here, so, I'll revise my
>> patch according to your vote.
>>
>> Thanks,
> 
> 


-- 
KaiGai Kohei <kai...@ak.jp.nec.com>
*** a/src/backend/catalog/heap.c
--- b/src/backend/catalog/heap.c
***************
*** 63,68 ****
--- 63,69 ----
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/inval.h"
  #include "utils/lsyscache.h"
  #include "utils/relcache.h"
***************
*** 1188,1193 **** heap_create_with_catalog(const char *relname,
--- 1189,1198 ----
  		}
  	}
  
+ 	/* Post creation of new relation */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							RelationRelationId, relid, 0);
+ 
  	/*
  	 * Store any supplied constraints and defaults.
  	 *
*** a/src/backend/catalog/pg_constraint.c
--- b/src/backend/catalog/pg_constraint.c
***************
*** 25,30 ****
--- 25,31 ----
  #include "utils/array.h"
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"
***************
*** 360,365 **** CreateConstraintEntry(const char *constraintName,
--- 361,370 ----
  										DEPENDENCY_NORMAL);
  	}
  
+ 	/* Post creation of a new constraint */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							ConstraintRelationId, conOid, 0);
+ 
  	return conOid;
  }
  
*** a/src/backend/catalog/pg_conversion.c
--- b/src/backend/catalog/pg_conversion.c
***************
*** 27,32 ****
--- 27,33 ----
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"
  #include "utils/tqual.h"
***************
*** 131,136 **** ConversionCreate(const char *conname, Oid connamespace,
--- 132,141 ----
  	recordDependencyOnOwner(ConversionRelationId, HeapTupleGetOid(tup),
  							conowner);
  
+ 	/* Post creation of a new conversion */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							ConversionRelationId, HeapTupleGetOid(tup), 0);
+ 
  	heap_freetuple(tup);
  	heap_close(rel, RowExclusiveLock);
  
*** a/src/backend/catalog/pg_namespace.c
--- b/src/backend/catalog/pg_namespace.c
***************
*** 19,24 ****
--- 19,25 ----
  #include "catalog/indexing.h"
  #include "catalog/pg_namespace.h"
  #include "utils/builtins.h"
+ #include "utils/hooks.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"
  
***************
*** 75,79 **** NamespaceCreate(const char *nspName, Oid ownerId)
--- 76,84 ----
  	/* Record dependency on owner */
  	recordDependencyOnOwner(NamespaceRelationId, nspoid, ownerId);
  
+ 	/* Post creation of new schema */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							NamespaceRelationId, nspoid, 0);
+ 
  	return nspoid;
  }
*** a/src/backend/catalog/pg_operator.c
--- b/src/backend/catalog/pg_operator.c
***************
*** 30,35 ****
--- 30,36 ----
  #include "parser/parse_oper.h"
  #include "utils/acl.h"
  #include "utils/builtins.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"
***************
*** 271,276 **** OperatorShellMake(const char *operatorName,
--- 272,281 ----
  	/* Add dependencies for the entry */
  	makeOperatorDependencies(tup);
  
+ 	/* Post creation of a new shell operator */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							OperatorRelationId, operatorObjectId, 0);
+ 
  	heap_freetuple(tup);
  
  	/*
***************
*** 539,544 **** OperatorCreate(const char *operatorName,
--- 544,553 ----
  	/* Add dependencies for the entry */
  	makeOperatorDependencies(tup);
  
+ 	/* Post creation of a new operator */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							OperatorRelationId, operatorObjectId, 0);
+ 
  	heap_close(pg_operator_desc, RowExclusiveLock);
  
  	/*
*** a/src/backend/catalog/pg_proc.c
--- b/src/backend/catalog/pg_proc.c
***************
*** 33,38 ****
--- 33,39 ----
  #include "tcop/tcopprot.h"
  #include "utils/acl.h"
  #include "utils/builtins.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/syscache.h"
  
***************
*** 614,619 **** ProcedureCreate(const char *procedureName,
--- 615,624 ----
  							  nnewmembers, newmembers);
  	}
  
+ 	/* Post creation of new procedure */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							ProcedureRelationId, retval, 0);
+ 
  	heap_freetuple(tup);
  
  	heap_close(rel, RowExclusiveLock);
*** a/src/backend/catalog/pg_type.c
--- b/src/backend/catalog/pg_type.c
***************
*** 28,33 ****
--- 28,34 ----
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"
***************
*** 155,160 **** TypeShellMake(const char *typeName, Oid typeNamespace, Oid ownerId)
--- 156,165 ----
  								 NULL,
  								 false);
  
+ 	/* Post creation of new shell type */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							TypeRelationId, typoid, 0);
+ 
  	/*
  	 * clean up and return the type-oid
  	 */
***************
*** 455,460 **** TypeCreate(Oid newTypeOid,
--- 460,469 ----
  								  NULL),
  								 rebuildDeps);
  
+ 	/* Post creation of new type */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							TypeRelationId, typeObjectId, 0);
+ 
  	/*
  	 * finish up
  	 */
*** a/src/backend/commands/dbcommands.c
--- b/src/backend/commands/dbcommands.c
***************
*** 53,58 ****
--- 53,59 ----
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/pg_locale.h"
  #include "utils/snapmgr.h"
***************
*** 571,576 **** createdb(const CreatedbStmt *stmt)
--- 572,581 ----
  	/* Create pg_shdepend entries for objects within database */
  	copyTemplateDependencies(src_dboid, dboid);
  
+ 	/* Post creation of new database */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							DatabaseRelationId, dboid, 0);
+ 
  	/*
  	 * Force a checkpoint before starting the copy. This will force dirty
  	 * buffers out to disk, to ensure source database is up-to-date on disk
*** a/src/backend/commands/functioncmds.c
--- b/src/backend/commands/functioncmds.c
***************
*** 57,62 ****
--- 57,63 ----
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
  #include "utils/guc.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"
***************
*** 1761,1766 **** CreateCast(CreateCastStmt *stmt)
--- 1762,1771 ----
  		recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
  	}
  
+ 	/* Post creation of a new cast */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							CastRelationId, myself.objectId, 0);
+ 
  	heap_freetuple(tuple);
  
  	heap_close(relation, RowExclusiveLock);
*** a/src/backend/commands/opclasscmds.c
--- b/src/backend/commands/opclasscmds.c
***************
*** 38,43 ****
--- 38,44 ----
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"
***************
*** 306,311 **** CreateOpFamily(char *amname, char *opfname, Oid namespaceoid, Oid amoid)
--- 307,316 ----
  	/* dependency on owner */
  	recordDependencyOnOwner(OperatorFamilyRelationId, opfamilyoid, GetUserId());
  
+ 	/* Post creation of a new operator family */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							OperatorFamilyRelationId, opfamilyoid, 0);
+ 
  	heap_close(rel, RowExclusiveLock);
  
  	return opfamilyoid;
***************
*** 693,698 **** DefineOpClass(CreateOpClassStmt *stmt)
--- 698,707 ----
  	/* dependency on owner */
  	recordDependencyOnOwner(OperatorClassRelationId, opclassoid, GetUserId());
  
+ 	/* Post creation of a new operator class */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							OperatorClassRelationId, opclassoid, 0);
+ 
  	heap_close(rel, RowExclusiveLock);
  }
  
*** a/src/backend/commands/proclang.c
--- b/src/backend/commands/proclang.c
***************
*** 32,37 ****
--- 32,38 ----
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"
***************
*** 425,430 **** create_proc_lang(const char *languageName, bool replace,
--- 426,435 ----
  		recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
  	}
  
+ 	/* Post creation of a new procedural language */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							LanguageRelationId, myself.objectId, 0);
+ 
  	heap_close(rel, RowExclusiveLock);
  }
  
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 69,74 ****
--- 69,75 ----
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/inval.h"
  #include "utils/lsyscache.h"
  #include "utils/memutils.h"
***************
*** 4024,4029 **** ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
--- 4025,4034 ----
  
  	heap_close(pgclass, RowExclusiveLock);
  
+ 	/* Post creation of new attribute */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							AttributeRelationId, myrelid, newattnum);
+ 
  	/* Make the attribute's catalog entry visible */
  	CommandCounterIncrement();
  
*** a/src/backend/commands/tablespace.c
--- b/src/backend/commands/tablespace.c
***************
*** 72,77 ****
--- 72,78 ----
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
  #include "utils/guc.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/memutils.h"
  #include "utils/rel.h"
***************
*** 333,338 **** CreateTableSpace(CreateTableSpaceStmt *stmt)
--- 334,343 ----
  	/* Record dependency on owner */
  	recordDependencyOnOwner(TableSpaceRelationId, tablespaceoid, ownerId);
  
+ 	/* Post creation of a new tablespace */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							TableSpaceRelationId, tablespaceoid, 0);
+ 
  	create_tablespace_directories(location, tablespaceoid);
  
  	/* Record the filesystem change in XLOG */
*** a/src/backend/commands/trigger.c
--- b/src/backend/commands/trigger.c
***************
*** 46,51 ****
--- 46,52 ----
  #include "utils/builtins.h"
  #include "utils/bytea.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/inval.h"
  #include "utils/lsyscache.h"
  #include "utils/memutils.h"
***************
*** 735,740 **** CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
--- 736,745 ----
  		recordDependencyOnExpr(&myself, whenClause, whenRtable,
  							   DEPENDENCY_NORMAL);
  
+ 	/* Post creation of a new trigger */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							TriggerRelationId, trigoid, 0);
+ 
  	/* Keep lock on target rel until end of xact */
  	heap_close(rel, NoLock);
  
*** a/src/backend/commands/tsearchcmds.c
--- b/src/backend/commands/tsearchcmds.c
***************
*** 42,47 ****
--- 42,48 ----
  #include "utils/builtins.h"
  #include "utils/catcache.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"
***************
*** 263,268 **** DefineTSParser(List *names, List *parameters)
--- 264,272 ----
  
  	makeParserDependencies(tup);
  
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							TSParserRelationId, prsOid, 0);
+ 
  	heap_freetuple(tup);
  
  	heap_close(prsRel, RowExclusiveLock);
***************
*** 563,568 **** DefineTSDictionary(List *names, List *parameters)
--- 567,575 ----
  
  	makeDictionaryDependencies(tup);
  
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							TSDictionaryRelationId, dictOid, 0);
+ 
  	heap_freetuple(tup);
  
  	heap_close(dictRel, RowExclusiveLock);
***************
*** 1050,1055 **** DefineTSTemplate(List *names, List *parameters)
--- 1057,1065 ----
  
  	makeTSTemplateDependencies(tup);
  
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							TSTemplateRelationId, dictOid, 0);
+ 
  	heap_freetuple(tup);
  
  	heap_close(tmplRel, RowExclusiveLock);
***************
*** 1440,1445 **** DefineTSConfiguration(List *names, List *parameters)
--- 1450,1458 ----
  
  	makeConfigurationDependencies(tup, false, mapRel);
  
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							TSConfigRelationId, cfgOid, 0);
+ 
  	heap_freetuple(tup);
  
  	if (mapRel)
*** a/src/backend/commands/user.c
--- b/src/backend/commands/user.c
***************
*** 30,35 ****
--- 30,36 ----
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/lsyscache.h"
  #include "utils/syscache.h"
  #include "utils/tqual.h"
***************
*** 402,407 **** CreateRole(CreateRoleStmt *stmt)
--- 403,412 ----
  				rolemembers, roleNamesToIds(rolemembers),
  				GetUserId(), false);
  
+ 	/* Post creation of a new role */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							AuthIdRelationId, roleid, 0);
+ 
  	/*
  	 * Close pg_authid, but keep lock till commit.
  	 */
*** a/src/backend/rewrite/rewriteDefine.c
--- b/src/backend/rewrite/rewriteDefine.c
***************
*** 29,34 ****
--- 29,35 ----
  #include "rewrite/rewriteSupport.h"
  #include "utils/acl.h"
  #include "utils/builtins.h"
+ #include "utils/hooks.h"
  #include "utils/inval.h"
  #include "utils/lsyscache.h"
  #include "utils/rel.h"
***************
*** 177,182 **** InsertRule(char *rulname,
--- 178,187 ----
  							   DEPENDENCY_NORMAL);
  	}
  
+ 	/* Post creation of a new rule */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							RewriteRelationId, rewriteObjectId, 0);
+ 
  	heap_close(pg_rewrite_desc, RowExclusiveLock);
  
  	return rewriteObjectId;
*** a/src/backend/storage/large_object/inv_api.c
--- b/src/backend/storage/large_object/inv_api.c
***************
*** 45,50 ****
--- 45,51 ----
  #include "miscadmin.h"
  #include "storage/large_object.h"
  #include "utils/fmgroids.h"
+ #include "utils/hooks.h"
  #include "utils/rel.h"
  #include "utils/resowner.h"
  #include "utils/snapmgr.h"
***************
*** 218,223 **** inv_create(Oid lobjId)
--- 219,228 ----
  	recordDependencyOnOwner(LargeObjectRelationId,
  							lobjId_new, GetUserId());
  
+ 	/* Post creation of a new large object */
+ 	InvokeObjectAccessHook0(OAT_POST_CREATE,
+ 							LargeObjectRelationId, lobjId_new, 0);
+ 
  	/*
  	 * Advance command counter to make new tuple visible to later operations.
  	 */
*** a/src/backend/utils/init/globals.c
--- b/src/backend/utils/init/globals.c
***************
*** 21,26 ****
--- 21,27 ----
  #include "libpq/pqcomm.h"
  #include "miscadmin.h"
  #include "storage/backendid.h"
+ #include "utils/hooks.h"
  
  
  ProtocolVersion FrontendProtocol;
***************
*** 117,119 **** int			VacuumCostBalance = 0;		/* working state for vacuum */
--- 118,126 ----
  bool		VacuumCostActive = false;
  
  int			GinFuzzySearchLimit = 0;
+ 
+ /*
+  * Hooks on object accesses what can be applied for external security
+  * providers and so on.
+  */
+ PGDLLIMPORT object_access_hook_type object_access_hook = NULL;
*** /dev/null
--- b/src/include/utils/hooks.h
***************
*** 0 ****
--- 1,48 ----
+ /*
+  * hooks.h
+  *
+  *		Definitions and introductions for object access hooks.
+  *		This hooks allows plugins to handle events on accesses
+  *		to database objects with user's query.
+  *
+  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
+  * Portions Copyright (c) 1994, Regents of the University of California
+  */
+ #ifndef HOOKS_H
+ #define HOOKS_H
+ 
+ typedef enum ObjectAccessType
+ {
+ 	OAT_POST_CREATE,	/* Post object creation */
+ } ObjectAccessType;
+ 
+ /*
+  * Introduction of every access types
+  * -----------------------------------
+  *
+  * OAT_POST_CREATE
+  *
+  * This access type shall be invoked just after object creation.
+  * Right now, it does not take any arguments except for object identifiers.
+  */
+ 
+ /*
+  * Definition of the object access hook
+  */
+ typedef void (*object_access_hook_type)(ObjectAccessType access,
+ 										Oid classId,
+ 										Oid objectId,
+ 										int subId);
+ 
+ extern PGDLLIMPORT object_access_hook_type object_access_hook;
+ 
+ /*
+  * Utility macros
+  */
+ #define InvokeObjectAccessHook0(access,classId,objectId,subId)			\
+ 	do {																\
+ 		if (object_access_hook)											\
+ 			(*object_access_hook)((access),(classId),(objectId),(subId)); \
+ 	} while(0)
+ 
+ #endif	/* HOOKS_H */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to