- You should write some regression tests for this functionality
- You should update the documentation
- Is there a reason why you've made the type of SQLCODE `text', rather than integer?
Pavel Stehule wrote:
+ fict_vars_sect :
+ {
+ plpgsql_ns_setlocal(false);
+ PLpgSQL_variable *var;
+ var = plpgsql_build_variable(strdup("sqlcode"), 0,
+ plpgsql_build_datatype(TEXTOID, -1), true); + $$.sqlcode_varno = var->dno;
+ var = plpgsql_build_variable(strdup("sqlerrm"), 0,
+ plpgsql_build_datatype(TEXTOID, -1), true);
This shouldn't be strdup'ing its first argument (and even if it needed to make a copy, it should use pstrdup). Also, my personal preference would be to implement this without creating a new production (i.e. just include it inline in the body of the pl_block production).
*** src.old/pl_exec.c 2005-02-24 02:11:40.000000000 +0100
--- src/pl_exec.c 2005-03-07 09:53:52.630243888 +0100
***************
*** 809,814 ****
--- 809,828 ----
int i;
int n;
+ /* setup SQLCODE and SQLERRM */
+ PLpgSQL_var *var;
+ + var = (PLpgSQL_var *) (estate->datums[block->sqlcode_varno]);
+ var->isnull = false;
+ var->freeval = false;
+ var->value = DirectFunctionCall1(textin, CStringGetDatum("00000"));
+ + var = (PLpgSQL_var *) (estate->datums[block->sqlerrm_varno]);
+ var->isnull = false;
+ var->freeval = false;
+ var->value = DirectFunctionCall1(textin, CStringGetDatum("Sucessful completion"));
`freeval' should be true, no? (Not sure it actually matters, but text is certainly not pass-by-value).
[...]*************** *** 918,923 **** --- 932,957 ----
+ var = (PLpgSQL_var *) (estate->datums[block->sqlcode_varno]);
+ var->value = DirectFunctionCall1(textin, CStringGetDatum(tbuf));
+ + var = (PLpgSQL_var *) (estate->datums[block->sqlerrm_varno]);
+ var->value = DirectFunctionCall1(textin, CStringGetDatum(edata->message));
You should probably pfree() the old values before replacing them.
-Neil
---------------------------(end of broadcast)--------------------------- TIP 4: Don't 'kill -9' the postmaster
