Excerpts from Pavel Stehule's message of mar feb 28 16:30:58 -0300 2012:
> Hello
>
> Dne 28. února 2012 17:48 Alvaro Herrera <[email protected]>
> napsal(a):
> >
> >
> > I have a few comments about this patch:
> >
> > I didn't like the fact that the checker calling infrastructure uses
> > SPI instead of just a FunctionCallN to call the checker function. I
> > think this should be easily avoidable.
>
> It is not possible - or it has not simple solution (I don't how to do
> it). PLpgSQL_checker is SRF function. SPI is used for processing
> returned resultset. I looked to pg source code, and I didn't find any
> other pattern than using SPI for SRF function call. It is probably
> possible, but it means some code duplication too. I invite any ideas.
It wasn't all that difficult -- see below. While at this, I have a
question: how attached you are to the current return format for CHECK
FUNCTION?
check function f1();
CHECK FUNCTION
-------------------------------------------------------------
In function: 'f1()'
error:42804:5:assignment:subscripted object is not an array
(2 rows)
It seems to me that it'd be trivial to make it look like this instead:
check function f1();
function | lineno | statement | sqlstate | message
| detail | hint | level | position | query
---------+--------+------------+----------+------------------------------------+--------+------+-------+----------+-------
f1() | 5 | assignment | 42804 | subscripted object is not an array
| | | error | |
(1 row)
This looks much nicer to me.
One thing we lose is the caret marking the position of the error -- but
I'm wondering if that really works well. I didn't test it but from the
code it looks to me like it'd misbehave if you had a multiline statement.
Opinions?
/*
* Search and execute the checker function.
*
* returns true, when checked function is valid
*/
static bool
CheckFunctionById(Oid funcOid, Oid relid, ArrayType *options,
bool fatal_errors, TupOutputState *tstate)
{
HeapTuple tup;
Form_pg_proc proc;
HeapTuple languageTuple;
Form_pg_language lanForm;
Oid languageChecker;
char *funcname;
int result;
tup = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcOid));
if (!HeapTupleIsValid(tup)) /* should not happen */
elog(ERROR, "cache lookup failed for function %u", funcOid);
proc = (Form_pg_proc) GETSTRUCT(tup);
languageTuple = SearchSysCache1(LANGOID,
ObjectIdGetDatum(proc->prolang));
Assert(HeapTupleIsValid(languageTuple));
lanForm = (Form_pg_language) GETSTRUCT(languageTuple);
languageChecker = lanForm->lanchecker;
funcname = format_procedure(funcOid);
/* We're all set to call the checker */
if (OidIsValid(languageChecker))
{
TupleDesc tupdesc;
Datum checkret;
FmgrInfo flinfo;
ReturnSetInfo rsinfo;
FunctionCallInfoData fcinfo;
/* create the tuple descriptor that the checker is supposed to
return */
tupdesc = CreateTemplateTupleDesc(10, false);
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "functionid",
REGPROCOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2, "lineno", INT4OID,
-1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 3, "statement",
TEXTOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 4, "sqlstate",
TEXTOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 5, "message", TEXTOID,
-1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6, "detail", TEXTOID,
-1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 7, "hint", TEXTOID,
-1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 8, "level", TEXTOID,
-1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 9, "position",
INT4OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 10, "query", TEXTOID,
-1, 0);
fmgr_info(languageChecker, &flinfo);
rsinfo.type = T_ReturnSetInfo;
rsinfo.econtext = CreateStandaloneExprContext();
rsinfo.expectedDesc = tupdesc;
rsinfo.allowedModes = (int) SFRM_Materialize;
/* returnMode is set by the checker, hopefully ... */
/* isDone is not relevant, since not using ValuePerCall */
rsinfo.setResult = NULL;
rsinfo.setDesc = NULL;
InitFunctionCallInfoData(fcinfo, &flinfo, 4, InvalidOid, NULL,
(Node *) &rsinfo);
fcinfo.arg[0] = ObjectIdGetDatum(funcOid);
fcinfo.arg[1] = ObjectIdGetDatum(relid);
fcinfo.arg[2] = PointerGetDatum(options);
fcinfo.arg[3] = BoolGetDatum(fatal_errors);
fcinfo.argnull[0] = false;
fcinfo.argnull[1] = false;
fcinfo.argnull[2] = false;
fcinfo.argnull[3] = false;
checkret = FunctionCallInvoke(&fcinfo);
if (rsinfo.returnMode != SFRM_Materialize)
elog(ERROR, "checker function didn't return a proper
return set");
/* XXX we have to do some checking on rsinfo.isDone and
checkret here */
if (rsinfo.setResult != NULL)
{
bool isnull;
StringInfoData str;
TupleTableSlot *slot =
MakeSingleTupleTableSlot(tupdesc);
initStringInfo(&str);
while (tuplestore_gettupleslot(rsinfo.setResult, true,
false, slot))
{
text *message = (text *)
DatumGetPointer(slot_getattr(slot, 5, &isnull));
resetStringInfo(&str);
appendStringInfo(&str, "got a message: %s",
text_to_cstring(message));
do_text_output_oneline(tstate, str.data);
}
pfree(str.data);
ExecDropSingleTupleTableSlot(slot);
}
}
pfree(funcname);
ReleaseSysCache(languageTuple);
ReleaseSysCache(tup);
return result;
}
--
Álvaro Herrera <[email protected]>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
