Modify error context callback functions to not assume that they can fetch
catalog entries via SearchSysCache and related operations. Although, at the time that these callbacks are called by elog.c, we have not officially aborted the current transaction, it still seems rather risky to initiate any new catalog fetches. In all these cases the needed information is readily available in the caller and so it's just a matter of a bit of extra notation to pass it to the callback. Per crash report from Dennis Koegel. I've concluded that the real fix for his problem is to clear the error context stack at entry to proc_exit, but it still seems like a good idea to make the callbacks a bit less fragile for other cases. Backpatch to 8.4. We could go further back, but the patch doesn't apply cleanly. In the absence of proof that this fixes something and isn't just paranoia, I'm not going to expend the effort.
This commit is contained in:
Tom Lane
parent
865b29540e
commit
a836abe9f6
|
|
@ -8,7 +8,7 @@
|
|||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.172 2010/02/26 02:00:37 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.173 2010/03/19 22:54:40 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -41,6 +41,12 @@ Datum fmgr_internal_validator(PG_FUNCTION_ARGS);
|
|||
Datum fmgr_c_validator(PG_FUNCTION_ARGS);
|
||||
Datum fmgr_sql_validator(PG_FUNCTION_ARGS);
|
||||
|
||||
typedef struct
|
||||
{
|
||||
char *proname;
|
||||
char *prosrc;
|
||||
} parse_error_callback_arg;
|
||||
|
||||
static void sql_function_parse_error_callback(void *arg);
|
||||
static int match_prosrc_to_query(const char *prosrc, const char *queryText,
|
||||
int cursorpos);
|
||||
|
|
@ -733,6 +739,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
|
|||
bool isnull;
|
||||
Datum tmp;
|
||||
char *prosrc;
|
||||
parse_error_callback_arg callback_arg;
|
||||
ErrorContextCallback sqlerrcontext;
|
||||
bool haspolyarg;
|
||||
int i;
|
||||
|
|
@ -782,8 +789,11 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
|
|||
/*
|
||||
* Setup error traceback support for ereport().
|
||||
*/
|
||||
callback_arg.proname = NameStr(proc->proname);
|
||||
callback_arg.prosrc = prosrc;
|
||||
|
||||
sqlerrcontext.callback = sql_function_parse_error_callback;
|
||||
sqlerrcontext.arg = tuple;
|
||||
sqlerrcontext.arg = (void *) &callback_arg;
|
||||
sqlerrcontext.previous = error_context_stack;
|
||||
error_context_stack = &sqlerrcontext;
|
||||
|
||||
|
|
@ -822,25 +832,14 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
|
|||
static void
|
||||
sql_function_parse_error_callback(void *arg)
|
||||
{
|
||||
HeapTuple tuple = (HeapTuple) arg;
|
||||
Form_pg_proc proc = (Form_pg_proc) GETSTRUCT(tuple);
|
||||
bool isnull;
|
||||
Datum tmp;
|
||||
char *prosrc;
|
||||
parse_error_callback_arg *callback_arg = (parse_error_callback_arg *) arg;
|
||||
|
||||
/* See if it's a syntax error; if so, transpose to CREATE FUNCTION */
|
||||
tmp = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_prosrc, &isnull);
|
||||
if (isnull)
|
||||
elog(ERROR, "null prosrc");
|
||||
prosrc = TextDatumGetCString(tmp);
|
||||
|
||||
if (!function_parse_error_transpose(prosrc))
|
||||
if (!function_parse_error_transpose(callback_arg->prosrc))
|
||||
{
|
||||
/* If it's not a syntax error, push info onto context stack */
|
||||
errcontext("SQL function \"%s\"", NameStr(proc->proname));
|
||||
errcontext("SQL function \"%s\"", callback_arg->proname);
|
||||
}
|
||||
|
||||
pfree(prosrc);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@
|
|||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.142 2010/02/26 02:00:41 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.143 2010/03/19 22:54:40 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -74,6 +74,7 @@ typedef struct execution_state
|
|||
*/
|
||||
typedef struct
|
||||
{
|
||||
char *fname; /* function name (for error msgs) */
|
||||
char *src; /* function body text (for error msgs) */
|
||||
|
||||
Oid *argtypes; /* resolved types of arguments */
|
||||
|
|
@ -228,6 +229,7 @@ init_sql_fcache(FmgrInfo *finfo, bool lazyEvalOK)
|
|||
bool isNull;
|
||||
|
||||
fcache = (SQLFunctionCachePtr) palloc0(sizeof(SQLFunctionCache));
|
||||
finfo->fn_extra = (void *) fcache;
|
||||
|
||||
/*
|
||||
* get the procedure tuple corresponding to the given function Oid
|
||||
|
|
@ -237,6 +239,11 @@ init_sql_fcache(FmgrInfo *finfo, bool lazyEvalOK)
|
|||
elog(ERROR, "cache lookup failed for function %u", foid);
|
||||
procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple);
|
||||
|
||||
/*
|
||||
* copy function name immediately for use by error reporting callback
|
||||
*/
|
||||
fcache->fname = pstrdup(NameStr(procedureStruct->proname));
|
||||
|
||||
/*
|
||||
* get the result type from the procedure tuple, and check for polymorphic
|
||||
* result type; if so, find out the actual result type.
|
||||
|
|
@ -361,8 +368,6 @@ init_sql_fcache(FmgrInfo *finfo, bool lazyEvalOK)
|
|||
lazyEvalOK);
|
||||
|
||||
ReleaseSysCache(procedureTuple);
|
||||
|
||||
finfo->fn_extra = (void *) fcache;
|
||||
}
|
||||
|
||||
/* Start up execution of one execution_state node */
|
||||
|
|
@ -877,37 +882,24 @@ sql_exec_error_callback(void *arg)
|
|||
{
|
||||
FmgrInfo *flinfo = (FmgrInfo *) arg;
|
||||
SQLFunctionCachePtr fcache = (SQLFunctionCachePtr) flinfo->fn_extra;
|
||||
HeapTuple func_tuple;
|
||||
Form_pg_proc functup;
|
||||
char *fn_name;
|
||||
int syntaxerrposition;
|
||||
|
||||
/* Need access to function's pg_proc tuple */
|
||||
func_tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(flinfo->fn_oid));
|
||||
if (!HeapTupleIsValid(func_tuple))
|
||||
return; /* shouldn't happen */
|
||||
functup = (Form_pg_proc) GETSTRUCT(func_tuple);
|
||||
fn_name = NameStr(functup->proname);
|
||||
/*
|
||||
* We can do nothing useful if init_sql_fcache() didn't get as far as
|
||||
* saving the function name
|
||||
*/
|
||||
if (fcache == NULL || fcache->fname == NULL)
|
||||
return;
|
||||
|
||||
/*
|
||||
* If there is a syntax error position, convert to internal syntax error
|
||||
*/
|
||||
syntaxerrposition = geterrposition();
|
||||
if (syntaxerrposition > 0)
|
||||
if (syntaxerrposition > 0 && fcache->src != NULL)
|
||||
{
|
||||
bool isnull;
|
||||
Datum tmp;
|
||||
char *prosrc;
|
||||
|
||||
tmp = SysCacheGetAttr(PROCOID, func_tuple, Anum_pg_proc_prosrc,
|
||||
&isnull);
|
||||
if (isnull)
|
||||
elog(ERROR, "null prosrc");
|
||||
prosrc = TextDatumGetCString(tmp);
|
||||
errposition(0);
|
||||
internalerrposition(syntaxerrposition);
|
||||
internalerrquery(prosrc);
|
||||
pfree(prosrc);
|
||||
internalerrquery(fcache->src);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -917,7 +909,7 @@ sql_exec_error_callback(void *arg)
|
|||
* ExecutorEnd are blamed on the appropriate query; see postquel_start and
|
||||
* postquel_end.)
|
||||
*/
|
||||
if (fcache)
|
||||
if (fcache->func_state)
|
||||
{
|
||||
execution_state *es;
|
||||
int query_num;
|
||||
|
|
@ -929,7 +921,7 @@ sql_exec_error_callback(void *arg)
|
|||
if (es->qd)
|
||||
{
|
||||
errcontext("SQL function \"%s\" statement %d",
|
||||
fn_name, query_num);
|
||||
fcache->fname, query_num);
|
||||
break;
|
||||
}
|
||||
es = es->next;
|
||||
|
|
@ -941,16 +933,18 @@ sql_exec_error_callback(void *arg)
|
|||
* couldn't identify a running query; might be function entry,
|
||||
* function exit, or between queries.
|
||||
*/
|
||||
errcontext("SQL function \"%s\"", fn_name);
|
||||
errcontext("SQL function \"%s\"", fcache->fname);
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
/* must have failed during init_sql_fcache() */
|
||||
errcontext("SQL function \"%s\" during startup", fn_name);
|
||||
/*
|
||||
* Assume we failed during init_sql_fcache(). (It's possible that
|
||||
* the function actually has an empty body, but in that case we may
|
||||
* as well report all errors as being "during startup".)
|
||||
*/
|
||||
errcontext("SQL function \"%s\" during startup", fcache->fname);
|
||||
}
|
||||
|
||||
ReleaseSysCache(func_tuple);
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@
|
|||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.286 2010/02/26 02:00:46 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.287 2010/03/19 22:54:41 tgl Exp $
|
||||
*
|
||||
* HISTORY
|
||||
* AUTHOR DATE MAJOR EVENT
|
||||
|
|
@ -71,6 +71,12 @@ typedef struct
|
|||
int sublevels_up;
|
||||
} substitute_actual_srf_parameters_context;
|
||||
|
||||
typedef struct
|
||||
{
|
||||
char *proname;
|
||||
char *prosrc;
|
||||
} inline_error_callback_arg;
|
||||
|
||||
static bool contain_agg_clause_walker(Node *node, void *context);
|
||||
static bool pull_agg_clause_walker(Node *node, List **context);
|
||||
static bool count_agg_clauses_walker(Node *node, AggClauseCounts *counts);
|
||||
|
|
@ -3678,6 +3684,7 @@ inline_function(Oid funcid, Oid result_type, List *args,
|
|||
bool modifyTargetList;
|
||||
MemoryContext oldcxt;
|
||||
MemoryContext mycxt;
|
||||
inline_error_callback_arg callback_arg;
|
||||
ErrorContextCallback sqlerrcontext;
|
||||
List *raw_parsetree_list;
|
||||
Query *querytree;
|
||||
|
|
@ -3705,15 +3712,6 @@ inline_function(Oid funcid, Oid result_type, List *args,
|
|||
if (pg_proc_aclcheck(funcid, GetUserId(), ACL_EXECUTE) != ACLCHECK_OK)
|
||||
return NULL;
|
||||
|
||||
/*
|
||||
* Setup error traceback support for ereport(). This is so that we can
|
||||
* finger the function that bad information came from.
|
||||
*/
|
||||
sqlerrcontext.callback = sql_inline_error_callback;
|
||||
sqlerrcontext.arg = func_tuple;
|
||||
sqlerrcontext.previous = error_context_stack;
|
||||
error_context_stack = &sqlerrcontext;
|
||||
|
||||
/*
|
||||
* Make a temporary memory context, so that we don't leak all the stuff
|
||||
* that parsing might create.
|
||||
|
|
@ -3725,6 +3723,27 @@ inline_function(Oid funcid, Oid result_type, List *args,
|
|||
ALLOCSET_DEFAULT_MAXSIZE);
|
||||
oldcxt = MemoryContextSwitchTo(mycxt);
|
||||
|
||||
/* Fetch the function body */
|
||||
tmp = SysCacheGetAttr(PROCOID,
|
||||
func_tuple,
|
||||
Anum_pg_proc_prosrc,
|
||||
&isNull);
|
||||
if (isNull)
|
||||
elog(ERROR, "null prosrc for function %u", funcid);
|
||||
src = TextDatumGetCString(tmp);
|
||||
|
||||
/*
|
||||
* Setup error traceback support for ereport(). This is so that we can
|
||||
* finger the function that bad information came from.
|
||||
*/
|
||||
callback_arg.proname = NameStr(funcform->proname);
|
||||
callback_arg.prosrc = src;
|
||||
|
||||
sqlerrcontext.callback = sql_inline_error_callback;
|
||||
sqlerrcontext.arg = (void *) &callback_arg;
|
||||
sqlerrcontext.previous = error_context_stack;
|
||||
error_context_stack = &sqlerrcontext;
|
||||
|
||||
/* Check for polymorphic arguments, and substitute actual arg types */
|
||||
argtypes = (Oid *) palloc(funcform->pronargs * sizeof(Oid));
|
||||
memcpy(argtypes, funcform->proargtypes.values,
|
||||
|
|
@ -3737,15 +3756,6 @@ inline_function(Oid funcid, Oid result_type, List *args,
|
|||
}
|
||||
}
|
||||
|
||||
/* Fetch and parse the function body */
|
||||
tmp = SysCacheGetAttr(PROCOID,
|
||||
func_tuple,
|
||||
Anum_pg_proc_prosrc,
|
||||
&isNull);
|
||||
if (isNull)
|
||||
elog(ERROR, "null prosrc for function %u", funcid);
|
||||
src = TextDatumGetCString(tmp);
|
||||
|
||||
/*
|
||||
* We just do parsing and parse analysis, not rewriting, because rewriting
|
||||
* will not affect table-free-SELECT-only queries, which is all that we
|
||||
|
|
@ -3965,30 +3975,19 @@ substitute_actual_parameters_mutator(Node *node,
|
|||
static void
|
||||
sql_inline_error_callback(void *arg)
|
||||
{
|
||||
HeapTuple func_tuple = (HeapTuple) arg;
|
||||
Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
|
||||
inline_error_callback_arg *callback_arg = (inline_error_callback_arg *) arg;
|
||||
int syntaxerrposition;
|
||||
|
||||
/* If it's a syntax error, convert to internal syntax error report */
|
||||
syntaxerrposition = geterrposition();
|
||||
if (syntaxerrposition > 0)
|
||||
{
|
||||
bool isnull;
|
||||
Datum tmp;
|
||||
char *prosrc;
|
||||
|
||||
tmp = SysCacheGetAttr(PROCOID, func_tuple, Anum_pg_proc_prosrc,
|
||||
&isnull);
|
||||
if (isnull)
|
||||
elog(ERROR, "null prosrc");
|
||||
prosrc = TextDatumGetCString(tmp);
|
||||
errposition(0);
|
||||
internalerrposition(syntaxerrposition);
|
||||
internalerrquery(prosrc);
|
||||
internalerrquery(callback_arg->prosrc);
|
||||
}
|
||||
|
||||
errcontext("SQL function \"%s\" during inlining",
|
||||
NameStr(funcform->proname));
|
||||
errcontext("SQL function \"%s\" during inlining", callback_arg->proname);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -4096,6 +4095,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
|
|||
bool modifyTargetList;
|
||||
MemoryContext oldcxt;
|
||||
MemoryContext mycxt;
|
||||
inline_error_callback_arg callback_arg;
|
||||
ErrorContextCallback sqlerrcontext;
|
||||
List *raw_parsetree_list;
|
||||
List *querytree_list;
|
||||
|
|
@ -4170,15 +4170,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
|
|||
return NULL;
|
||||
}
|
||||
|
||||
/*
|
||||
* Setup error traceback support for ereport(). This is so that we can
|
||||
* finger the function that bad information came from.
|
||||
*/
|
||||
sqlerrcontext.callback = sql_inline_error_callback;
|
||||
sqlerrcontext.arg = func_tuple;
|
||||
sqlerrcontext.previous = error_context_stack;
|
||||
error_context_stack = &sqlerrcontext;
|
||||
|
||||
/*
|
||||
* Make a temporary memory context, so that we don't leak all the stuff
|
||||
* that parsing might create.
|
||||
|
|
@ -4190,6 +4181,27 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
|
|||
ALLOCSET_DEFAULT_MAXSIZE);
|
||||
oldcxt = MemoryContextSwitchTo(mycxt);
|
||||
|
||||
/* Fetch the function body */
|
||||
tmp = SysCacheGetAttr(PROCOID,
|
||||
func_tuple,
|
||||
Anum_pg_proc_prosrc,
|
||||
&isNull);
|
||||
if (isNull)
|
||||
elog(ERROR, "null prosrc for function %u", func_oid);
|
||||
src = TextDatumGetCString(tmp);
|
||||
|
||||
/*
|
||||
* Setup error traceback support for ereport(). This is so that we can
|
||||
* finger the function that bad information came from.
|
||||
*/
|
||||
callback_arg.proname = NameStr(funcform->proname);
|
||||
callback_arg.prosrc = src;
|
||||
|
||||
sqlerrcontext.callback = sql_inline_error_callback;
|
||||
sqlerrcontext.arg = (void *) &callback_arg;
|
||||
sqlerrcontext.previous = error_context_stack;
|
||||
error_context_stack = &sqlerrcontext;
|
||||
|
||||
/*
|
||||
* Run eval_const_expressions on the function call. This is necessary to
|
||||
* ensure that named-argument notation is converted to positional notation
|
||||
|
|
@ -4220,15 +4232,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
|
|||
}
|
||||
}
|
||||
|
||||
/* Fetch and parse the function body */
|
||||
tmp = SysCacheGetAttr(PROCOID,
|
||||
func_tuple,
|
||||
Anum_pg_proc_prosrc,
|
||||
&isNull);
|
||||
if (isNull)
|
||||
elog(ERROR, "null prosrc for function %u", func_oid);
|
||||
src = TextDatumGetCString(tmp);
|
||||
|
||||
/*
|
||||
* Parse, analyze, and rewrite (unlike inline_function(), we can't skip
|
||||
* rewriting here). We can fail as soon as we find more than one query,
|
||||
|
|
|
|||
Loading…
Reference in New Issue