On Sun, Oct 31, 2010 at 12:00, Andrew Dunstan <and...@dunslane.net> wrote:
> On 10/31/2010 11:44 AM, Tom Lane wrote:
>> Good catch, patch reverted (and regression test added).
>
> Well, I guess that answers the question of why we needed it, which nobody
> could answer before. I'm not sure I exactly understand what's going on here,
> though - I guess I need to look at it closer. At least I think we need a
> code comment on why the trigger flag is needed as part of the hash key.
The stack trace is:
#0 0x0000000000000000 in ?? ()
#1 0x00000000006c18e9 in InputFunctionCall (flinfo=0x2a039a0,
str=0x0, typioparam=0, typmod=-1)
#2 0x00007ff6d2bdf950 in plperl_func_handler (fcinfo=0x7fff4743bec0)
at plperl.c:1729
which happens because prodesc->result_in_func.fn_addr (flinfo) is
NULL. That happens because when we are a trigger we don't setup
input/output conversion. And with the change we get the same
proc_desc for triggers and non triggers, so if the trigger function
gets called first, any call to the direct function will use the same
proc_desc with the wrong input/out conversion.
There is a check so that trigger functions can not be called as plain
functions, but it only gets called when we do not have an entry in
plperl_proc_hash. I think just moving that up, something the like the
attached should be enough. Thoughts?
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 270e9f7..e269c2a 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -113,7 +113,7 @@ typedef struct plperl_proc_desc
/**********************************************************************
* For speedy lookup, we maintain a hash table mapping from
- * function OID + trigger flag + user OID to plperl_proc_desc pointers.
+ * function OID + user OID to plperl_proc_desc pointers.
* The reason the plperl_proc_desc struct isn't directly part of the hash
* entry is to simplify recovery from errors during compile_plperl_function.
*
@@ -127,11 +127,6 @@ typedef struct plperl_proc_desc
typedef struct plperl_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 */
} plperl_proc_key;
@@ -1958,9 +1953,20 @@ compile_plperl_function(Oid fn_oid, bool is_trigger)
plperl_error_context.arg = NameStr(procStruct->proname);
error_context_stack = &plperl_error_context;
+ /*
+ * if we are a trigger, but not called as one, bail out. otherwise we
+ * might have the wrong (or no) input/output conversion setup.
+ * We need to check here because trigger or not we would have the same
+ * plperl_proc_hash entry
+ */
+ if(!is_trigger && procStruct->prorettype == TRIGGEROID)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("trigger functions can only be called "
+ "as triggers")));
+
/* Try to find function in plperl_proc_hash */
proc_key.proc_id = fn_oid;
- proc_key.is_trigger = is_trigger;
proc_key.user_id = GetUserId();
proc_ptr = hash_search(plperl_proc_hash, &proc_key,
@@ -2057,15 +2063,6 @@ compile_plperl_function(Oid fn_oid, bool is_trigger)
if (procStruct->prorettype == VOIDOID ||
procStruct->prorettype == RECORDOID)
/* okay */ ;
- else if (procStruct->prorettype == TRIGGEROID)
- {
- free(prodesc->proname);
- free(prodesc);
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("trigger functions can only be called "
- "as triggers")));
- }
else
{
free(prodesc->proname);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
