On Mon, Nov 1, 2010 at 16:59, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Alex Hunsaker <bada...@gmail.com> writes:
>> Speaking of which, pltcl stores the trigger reloid instead of a flag
>> (it also uses tg_reloid in the internal proname).  It seems a tad
>> excessive to have one function *per* trigger table.
>
> Surely, removing the internal name's dependency on the istrigger flag is
> wrong.  If you're going to maintain separate hash entries at the pltcl
> level, why would you want to risk collisions underneath that?

Good catch.  I was basing it off plperl which uses the same proname
for both (sprintf(subname, %s__%u", prodesc->proname, fn_oid)).  Its
OK for plperl because when we compile we save a reference to it and
use that directly (more or less).  The name does not really matter.
*** a/src/pl/tcl/pltcl.c
--- b/src/pl/tcl/pltcl.c
***************
*** 137,143 **** typedef struct pltcl_query_desc
  
  /**********************************************************************
   * For speedy lookup, we maintain a hash table mapping from
!  * function OID + trigger OID + user OID to pltcl_proc_desc pointers.
   * The reason the pltcl_proc_desc struct isn't directly part of the hash
   * entry is to simplify recovery from errors during compile_pltcl_function.
   *
--- 137,143 ----
  
  /**********************************************************************
   * For speedy lookup, we maintain a hash table mapping from
!  * function OID + trigger flag + user OID to pltcl_proc_desc pointers.
   * The reason the pltcl_proc_desc struct isn't directly part of the hash
   * entry is to simplify recovery from errors during compile_pltcl_function.
   *
***************
*** 149,155 **** typedef struct pltcl_query_desc
  typedef struct pltcl_proc_key
  {
  	Oid			proc_id;				/* Function OID */
! 	Oid			trig_id;				/* Trigger OID, or 0 if not trigger */
  	Oid			user_id;				/* User calling the function, or 0 */
  } pltcl_proc_key;
  
--- 149,159 ----
  typedef struct pltcl_proc_key
  {
  	Oid			proc_id;				/* Function OID */
! 	/*
! 	 * is_trigger is really a bool, but declare as Oid to ensure this struct
! 	 * contains no padding
! 	 */
! 	Oid			is_trigger;				/* is it a trigger function? */
  	Oid			user_id;				/* User calling the function, or 0 */
  } pltcl_proc_key;
  
***************
*** 1172,1178 **** compile_pltcl_function(Oid fn_oid, Oid tgreloid, bool pltrusted)
  
  	/* Try to find function in pltcl_proc_htab */
  	proc_key.proc_id = fn_oid;
! 	proc_key.trig_id = tgreloid;
  	proc_key.user_id = pltrusted ? GetUserId() : InvalidOid;
  
  	proc_ptr = hash_search(pltcl_proc_htab, &proc_key,
--- 1176,1182 ----
  
  	/* Try to find function in pltcl_proc_htab */
  	proc_key.proc_id = fn_oid;
! 	proc_key.is_trigger = OidIsValid(tgreloid);
  	proc_key.user_id = pltrusted ? GetUserId() : InvalidOid;
  
  	proc_ptr = hash_search(pltcl_proc_htab, &proc_key,
***************
*** 1228,1241 **** compile_pltcl_function(Oid fn_oid, Oid tgreloid, bool pltrusted)
  		int			tcl_rc;
  
  		/************************************************************
! 		 * Build our internal proc name from the functions Oid + trigger Oid
! 		 ************************************************************/
! 		if (!is_trigger)
! 			snprintf(internal_proname, sizeof(internal_proname),
! 					 "__PLTcl_proc_%u", fn_oid);
! 		else
! 			snprintf(internal_proname, sizeof(internal_proname),
! 					 "__PLTcl_proc_%u_trigger_%u", fn_oid, tgreloid);
  
  		/************************************************************
  		 * Allocate a new procedure description block
--- 1232,1248 ----
  		int			tcl_rc;
  
  		/************************************************************
! 		* Build our internal proc name from the functions Oid.  Append
! 		* trigger when appropriate in case they try to manually call
! 		* the trigger and vice versa. (otherwise we might overwrite the
! 		* trigger procedure in TCL's namespace)
! 		************************************************************/
! 	       if (!is_trigger)
! 		       snprintf(internal_proname, sizeof(internal_proname),
! 					"__PLTcl_proc_%u", fn_oid);
! 	       else
! 		       snprintf(internal_proname, sizeof(internal_proname),
! 					"__PLTcl_proc_%u_trigger", fn_oid);
  
  		/************************************************************
  		 * Allocate a new procedure description block
-- 
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