On 15/09/2013 04:02, Peter Eisentraut wrote:
On Fri, 2013-09-13 at 23:56 +0200, Marko Tiikkaja wrote:
Attached is a patch for optionally printing more information on STRICT
failures in PL/PgSQL:
Please fix the tabs in the SGML files.
Oops. Thanks.
Attached an updated patch to fix the tabs and to change this to a
compile-time option. Now it's not possible to flexibly disable the
feature during runtime, but I think that's fine.
Regards,
Marko Tiikkaja
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 1077,1082 **** END;
--- 1077,1105 ----
</para>
<para>
+ If <literal>print_strict_params</> is enabled for the function,
+ you will get information about the parameters passed to the
+ query in the <literal>DETAIL</> part of the error message produced
+ when the requirements of STRICT are not met. You can change this
+ setting on a system-wide basis by setting
+ <varname>plpgsql.print_strict_params</>, though only subsequent
+ function compilations will be affected. You can also enable it
+ on a per-function basis by using a compiler option:
+ <programlisting>
+ CREATE FUNCTION get_userid(username text) RETURNS int
+ AS $$
+ #print_strict_params on
+ DECLARE
+ userid int;
+ BEGIN
+ SELECT users.userid INTO STRICT userid WHERE users.username =
get_userid.username;
+ RETURN userid;
+ END
+ $$ LANGUAGE plpgsql;
+ </programlisting>
+ </para>
+
+ <para>
For <command>INSERT</>/<command>UPDATE</>/<command>DELETE</> with
<literal>RETURNING</>, <application>PL/pgSQL</application> reports
an error for more than one returned row, even when
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
***************
*** 351,356 **** do_compile(FunctionCallInfo fcinfo,
--- 351,357 ----
function->fn_cxt = func_cxt;
function->out_param_varno = -1; /* set up for no OUT param */
function->resolve_option = plpgsql_variable_conflict;
+ function->print_strict_params = plpgsql_print_strict_params;
if (is_dml_trigger)
function->fn_is_trigger = PLPGSQL_DML_TRIGGER;
***************
*** 847,852 **** plpgsql_compile_inline(char *proc_source)
--- 848,854 ----
function->fn_cxt = func_cxt;
function->out_param_varno = -1; /* set up for no OUT param */
function->resolve_option = plpgsql_variable_conflict;
+ function->print_strict_params = plpgsql_print_strict_params;
plpgsql_ns_init();
plpgsql_ns_push(func_name);
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***************
*** 139,144 **** static void plpgsql_estate_setup(PLpgSQL_execstate *estate,
--- 139,150 ----
ReturnSetInfo *rsi);
static void exec_eval_cleanup(PLpgSQL_execstate *estate);
+ static char *exec_get_query_params(PLpgSQL_execstate *estate,
+ const
PLpgSQL_expr *expr);
+ static char *exec_get_dynquery_params(PLpgSQL_execstate *estate,
+ const
PreparedParamsData *ppd);
+
+
static void exec_prepare_plan(PLpgSQL_execstate *estate,
PLpgSQL_expr *expr, int cursorOptions);
static bool exec_simple_check_node(Node *node);
***************
*** 3226,3231 **** exec_prepare_plan(PLpgSQL_execstate *estate,
--- 3232,3310 ----
exec_simple_check_plan(expr);
}
+ static char *
+ exec_get_query_params(PLpgSQL_execstate *estate,
+ const PLpgSQL_expr *expr)
+ {
+ int paramno;
+ int dno;
+ StringInfoData paramstr;
+ Bitmapset *tmpset;
+
+ if (!expr->paramnos)
+ return "(no parameters)";
+
+ initStringInfo(¶mstr);
+ tmpset = bms_copy(expr->paramnos);
+ paramno = 1;
+ while ((dno = bms_first_member(tmpset)) >= 0)
+ {
+ Datum paramdatum;
+ Oid paramtypeid;
+ bool paramisnull;
+ int32 paramtypmod;
+ PLpgSQL_var *curvar;
+
+ curvar = (PLpgSQL_var *) estate->datums[dno];
+
+ exec_eval_datum(estate, (PLpgSQL_datum *) curvar, ¶mtypeid,
+ ¶mtypmod, ¶mdatum,
¶misnull);
+
+ if (paramno > 1)
+ appendStringInfo(¶mstr, ", ");
+
+ if (paramisnull)
+ appendStringInfo(¶mstr, "%s = NULL",
curvar->refname);
+ else
+ {
+ char *value = convert_value_to_string(estate,
paramdatum, paramtypeid);
+ appendStringInfo(¶mstr, "%s = '%s'",
curvar->refname, value);
+ }
+
+ paramno++;
+ }
+ bms_free(tmpset);
+
+ return paramstr.data;
+ }
+
+ static char *
+ exec_get_dynquery_params(PLpgSQL_execstate *estate,
+ const PreparedParamsData *ppd)
+ {
+ int i;
+ StringInfoData paramstr;
+
+ if (!ppd)
+ return "(no parameters)";
+
+ initStringInfo(¶mstr);
+ for (i = 0; i < ppd->nargs; ++i)
+ {
+ if (i > 0)
+ appendStringInfoString(¶mstr, ", ");
+
+ if (ppd->nulls[i] == 'n')
+ appendStringInfo(¶mstr, "$%d = NULL", i+1);
+ else
+ {
+ char *value = convert_value_to_string(estate,
ppd->values[i], ppd->types[i]);
+ appendStringInfo(¶mstr, "$%d = '%s'", i+1, value);
+ }
+ }
+
+ return paramstr.data;
+ }
/* ----------
* exec_stmt_execsql Execute an SQL statement (possibly with
INTO).
***************
*** 3391,3408 **** exec_stmt_execsql(PLpgSQL_execstate *estate,
if (n == 0)
{
if (stmt->strict)
ereport(ERROR,
(errcode(ERRCODE_NO_DATA_FOUND),
! errmsg("query returned no
rows")));
/* set the target to NULL(s) */
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
}
else
{
if (n > 1 && (stmt->strict || stmt->mod_stmt))
ereport(ERROR,
(errcode(ERRCODE_TOO_MANY_ROWS),
! errmsg("query returned more
than one row")));
/* Put the first result row into the target */
exec_move_row(estate, rec, row, tuptab->vals[0],
tuptab->tupdesc);
}
--- 3470,3509 ----
if (n == 0)
{
if (stmt->strict)
+ {
+ char *errdetail;
+
+ if (estate->func->print_strict_params)
+ errdetail =
exec_get_query_params(estate, expr);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_NO_DATA_FOUND),
! errmsg("query returned no
rows"),
! errdetail ?
!
errdetail_internal("%s", errdetail) : 0));
! }
/* set the target to NULL(s) */
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
}
else
{
if (n > 1 && (stmt->strict || stmt->mod_stmt))
+ {
+ char *errdetail;
+
+ if (estate->func->print_strict_params)
+ errdetail =
exec_get_query_params(estate, expr);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_TOO_MANY_ROWS),
! errmsg("query returned more
than one row"),
! errdetail ?
!
errdetail_internal("%s", errdetail) : 0));
! }
/* Put the first result row into the target */
exec_move_row(estate, rec, row, tuptab->vals[0],
tuptab->tupdesc);
}
***************
*** 3442,3447 **** exec_stmt_dynexecute(PLpgSQL_execstate *estate,
--- 3543,3549 ----
Oid restype;
char *querystr;
int exec_res;
+ PreparedParamsData *ppd = NULL;
/*
* First we evaluate the string expression after the EXECUTE keyword.
Its
***************
*** 3466,3479 **** exec_stmt_dynexecute(PLpgSQL_execstate *estate,
*/
if (stmt->params)
{
- PreparedParamsData *ppd;
-
ppd = exec_eval_using_params(estate, stmt->params);
exec_res = SPI_execute_with_args(querystr,
ppd->nargs, ppd->types,
ppd->values, ppd->nulls,
estate->readonly_func, 0);
- free_params_data(ppd);
}
else
exec_res = SPI_execute(querystr, estate->readonly_func, 0);
--- 3568,3578 ----
***************
*** 3565,3582 **** exec_stmt_dynexecute(PLpgSQL_execstate *estate,
if (n == 0)
{
if (stmt->strict)
ereport(ERROR,
(errcode(ERRCODE_NO_DATA_FOUND),
! errmsg("query returned no
rows")));
/* set the target to NULL(s) */
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
}
else
{
if (n > 1 && stmt->strict)
ereport(ERROR,
(errcode(ERRCODE_TOO_MANY_ROWS),
! errmsg("query returned more
than one row")));
/* Put the first result row into the target */
exec_move_row(estate, rec, row, tuptab->vals[0],
tuptab->tupdesc);
}
--- 3664,3704 ----
if (n == 0)
{
if (stmt->strict)
+ {
+ char *errdetail;
+
+ if (estate->func->print_strict_params)
+ errdetail =
exec_get_dynquery_params(estate, ppd);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_NO_DATA_FOUND),
! errmsg("query returned no
rows"),
! errdetail ?
!
errdetail_internal("%s", errdetail) : 0));
! }
/* set the target to NULL(s) */
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
}
else
{
if (n > 1 && stmt->strict)
+ {
+ char *errdetail;
+
+ if (estate->func->print_strict_params)
+ errdetail =
exec_get_dynquery_params(estate, ppd);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_TOO_MANY_ROWS),
! errmsg("query returned more
than one row"),
! errdetail ?
!
errdetail_internal("%s", errdetail) : 0));
! }
!
/* Put the first result row into the target */
exec_move_row(estate, rec, row, tuptab->vals[0],
tuptab->tupdesc);
}
***************
*** 3592,3597 **** exec_stmt_dynexecute(PLpgSQL_execstate *estate,
--- 3714,3722 ----
*/
}
+ if (ppd)
+ free_params_data(ppd);
+
/* Release any result from SPI_execute, as well as the querystring */
SPI_freetuptable(SPI_tuptable);
pfree(querystr);
*** a/src/pl/plpgsql/src/pl_gram.y
--- b/src/pl/plpgsql/src/pl_gram.y
***************
*** 214,219 **** static List *read_raise_options(void);
--- 214,221 ----
%type <keyword> unreserved_keyword
+ %type <boolean> boolean_setting
+
/*
* Basic non-keyword token types. These are hard-wired into the core lexer.
***************
*** 299,304 **** static List *read_raise_options(void);
--- 301,308 ----
%token <keyword> K_NOT
%token <keyword> K_NOTICE
%token <keyword> K_NULL
+ %token <keyword> K_OFF
+ %token <keyword> K_ON
%token <keyword> K_OPEN
%token <keyword> K_OPTION
%token <keyword> K_OR
***************
*** 308,313 **** static List *read_raise_options(void);
--- 312,318 ----
%token <keyword> K_PG_EXCEPTION_CONTEXT
%token <keyword> K_PG_EXCEPTION_DETAIL
%token <keyword> K_PG_EXCEPTION_HINT
+ %token <keyword> K_PRINT_STRICT_PARAMS
%token <keyword> K_PRIOR
%token <keyword> K_QUERY
%token <keyword> K_RAISE
***************
*** 354,359 **** comp_option : '#' K_OPTION K_DUMP
--- 359,368 ----
{
plpgsql_DumpExecTree = true;
}
+ | '#' K_PRINT_STRICT_PARAMS boolean_setting
+ {
+
plpgsql_curr_compile->print_strict_params = $3;
+ }
| '#' K_VARIABLE_CONFLICT K_ERROR
{
plpgsql_curr_compile->resolve_option = PLPGSQL_RESOLVE_ERROR;
***************
*** 368,373 **** comp_option : '#' K_OPTION K_DUMP
--- 377,387 ----
}
;
+ boolean_setting : K_ON
+ { $$ = true; }
+ | K_OFF
+ { $$ = false; }
+
opt_semi :
| ';'
;
*** a/src/pl/plpgsql/src/pl_handler.c
--- b/src/pl/plpgsql/src/pl_handler.c
***************
*** 37,42 **** static const struct config_enum_entry
variable_conflict_options[] = {
--- 37,44 ----
int plpgsql_variable_conflict = PLPGSQL_RESOLVE_ERROR;
+ bool plpgsql_print_strict_params = false;
+
/* Hook for plugins */
PLpgSQL_plugin **plugin_ptr = NULL;
***************
*** 66,71 **** _PG_init(void)
--- 68,81 ----
PGC_SUSET, 0,
NULL, NULL, NULL);
+ DefineCustomBoolVariable("plpgsql.print_strict_params",
+ gettext_noop("Print
information about parameters in the DETAIL part of the error messages generated
on INTO .. STRICT failures."),
+ NULL,
+
&plpgsql_print_strict_params,
+ false,
+ PGC_USERSET, 0,
+ NULL, NULL, NULL);
+
EmitWarningsOnPlaceholders("plpgsql");
plpgsql_HashTableInit();
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
***************
*** 134,145 **** static const ScanKeyword unreserved_keywords[] = {
--- 134,148 ----
PG_KEYWORD("next", K_NEXT, UNRESERVED_KEYWORD)
PG_KEYWORD("no", K_NO, UNRESERVED_KEYWORD)
PG_KEYWORD("notice", K_NOTICE, UNRESERVED_KEYWORD)
+ PG_KEYWORD("off", K_OFF, UNRESERVED_KEYWORD)
+ PG_KEYWORD("on", K_ON, UNRESERVED_KEYWORD)
PG_KEYWORD("option", K_OPTION, UNRESERVED_KEYWORD)
PG_KEYWORD("pg_context", K_PG_CONTEXT, UNRESERVED_KEYWORD)
PG_KEYWORD("pg_datatype_name", K_PG_DATATYPE_NAME, UNRESERVED_KEYWORD)
PG_KEYWORD("pg_exception_context", K_PG_EXCEPTION_CONTEXT,
UNRESERVED_KEYWORD)
PG_KEYWORD("pg_exception_detail", K_PG_EXCEPTION_DETAIL,
UNRESERVED_KEYWORD)
PG_KEYWORD("pg_exception_hint", K_PG_EXCEPTION_HINT, UNRESERVED_KEYWORD)
+ PG_KEYWORD("print_strict_params", K_PRINT_STRICT_PARAMS,
UNRESERVED_KEYWORD)
PG_KEYWORD("prior", K_PRIOR, UNRESERVED_KEYWORD)
PG_KEYWORD("query", K_QUERY, UNRESERVED_KEYWORD)
PG_KEYWORD("relative", K_RELATIVE, UNRESERVED_KEYWORD)
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 737,742 **** typedef struct PLpgSQL_function
--- 737,744 ----
PLpgSQL_resolve_option resolve_option;
+ bool print_strict_params;
+
int ndatums;
PLpgSQL_datum **datums;
PLpgSQL_stmt_block *action;
***************
*** 873,878 **** extern IdentifierLookup plpgsql_IdentifierLookup;
--- 875,882 ----
extern int plpgsql_variable_conflict;
+ extern bool plpgsql_print_strict_params;
+
extern bool plpgsql_check_syntax;
extern bool plpgsql_DumpExecTree;
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
***************
*** 3104,3109 **** select footest();
--- 3104,3215 ----
ERROR: query returned more than one row
CONTEXT: PL/pgSQL function footest() line 5 at EXECUTE statement
drop function footest();
+ -- test printing parameters after failure due to STRICT
+ set plpgsql.print_strict_params to true;
+ create or replace function footest() returns void as $$
+ declare
+ x record;
+ p1 int := 2;
+ p3 text := 'foo';
+ begin
+ -- no rows
+ select * from foo where f1 = p1 and f1::text = p3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+ select footest();
+ ERROR: query returned no rows
+ DETAIL: p1 = '2', p3 = 'foo'
+ CONTEXT: PL/pgSQL function footest() line 8 at SQL statement
+ create or replace function footest() returns void as $$
+ declare
+ x record;
+ p1 int := 2;
+ p3 text := 'foo';
+ begin
+ -- too many rows
+ select * from foo where f1 > p1 or f1::text = p3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+ select footest();
+ ERROR: query returned more than one row
+ DETAIL: p1 = '2', p3 = 'foo'
+ CONTEXT: PL/pgSQL function footest() line 8 at SQL statement
+ create or replace function footest() returns void as $$
+ declare x record;
+ begin
+ -- too many rows, no params
+ select * from foo where f1 > 3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+ select footest();
+ ERROR: query returned more than one row
+ DETAIL: (no parameters)
+ CONTEXT: PL/pgSQL function footest() line 5 at SQL statement
+ create or replace function footest() returns void as $$
+ declare x record;
+ begin
+ -- no rows
+ execute 'select * from foo where f1 = $1 or f1::text = $2' using 0, 'foo'
into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+ select footest();
+ ERROR: query returned no rows
+ DETAIL: $1 = '0', $2 = 'foo'
+ CONTEXT: PL/pgSQL function footest() line 5 at EXECUTE statement
+ create or replace function footest() returns void as $$
+ declare x record;
+ begin
+ -- too many rows
+ execute 'select * from foo where f1 > $1' using 1 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+ select footest();
+ ERROR: query returned more than one row
+ DETAIL: $1 = '1'
+ CONTEXT: PL/pgSQL function footest() line 5 at EXECUTE statement
+ create or replace function footest() returns void as $$
+ declare x record;
+ begin
+ -- too many rows, no parameters
+ execute 'select * from foo where f1 > 3' into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+ select footest();
+ ERROR: query returned more than one row
+ DETAIL: (no parameters)
+ CONTEXT: PL/pgSQL function footest() line 5 at EXECUTE statement
+ create or replace function footest() returns void as $$
+ -- override the global
+ #print_strict_params off
+ declare
+ x record;
+ p1 int := 2;
+ p3 text := 'foo';
+ begin
+ -- too many rows
+ select * from foo where f1 > p1 or f1::text = p3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+ select footest();
+ ERROR: query returned more than one row
+ CONTEXT: PL/pgSQL function footest() line 10 at SQL statement
+ reset plpgsql.print_strict_params;
+ create or replace function footest() returns void as $$
+ -- override the global
+ #print_strict_params on
+ declare
+ x record;
+ p1 int := 2;
+ p3 text := 'foo';
+ begin
+ -- too many rows
+ select * from foo where f1 > p1 or f1::text = p3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+ select footest();
+ ERROR: query returned more than one row
+ DETAIL: p1 = '2', p3 = 'foo'
+ CONTEXT: PL/pgSQL function footest() line 10 at SQL statement
-- test scrollable cursor support
create function sc_test() returns setof integer as $$
declare
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
***************
*** 2587,2592 **** select footest();
--- 2587,2694 ----
drop function footest();
+ -- test printing parameters after failure due to STRICT
+
+ set plpgsql.print_strict_params to true;
+
+ create or replace function footest() returns void as $$
+ declare
+ x record;
+ p1 int := 2;
+ p3 text := 'foo';
+ begin
+ -- no rows
+ select * from foo where f1 = p1 and f1::text = p3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+
+ select footest();
+
+ create or replace function footest() returns void as $$
+ declare
+ x record;
+ p1 int := 2;
+ p3 text := 'foo';
+ begin
+ -- too many rows
+ select * from foo where f1 > p1 or f1::text = p3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+
+ select footest();
+
+ create or replace function footest() returns void as $$
+ declare x record;
+ begin
+ -- too many rows, no params
+ select * from foo where f1 > 3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+
+ select footest();
+
+ create or replace function footest() returns void as $$
+ declare x record;
+ begin
+ -- no rows
+ execute 'select * from foo where f1 = $1 or f1::text = $2' using 0, 'foo'
into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+
+ select footest();
+
+ create or replace function footest() returns void as $$
+ declare x record;
+ begin
+ -- too many rows
+ execute 'select * from foo where f1 > $1' using 1 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+
+ select footest();
+
+ create or replace function footest() returns void as $$
+ declare x record;
+ begin
+ -- too many rows, no parameters
+ execute 'select * from foo where f1 > 3' into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+
+ select footest();
+
+ create or replace function footest() returns void as $$
+ -- override the global
+ #print_strict_params off
+ declare
+ x record;
+ p1 int := 2;
+ p3 text := 'foo';
+ begin
+ -- too many rows
+ select * from foo where f1 > p1 or f1::text = p3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+
+ select footest();
+
+ reset plpgsql.print_strict_params;
+
+ create or replace function footest() returns void as $$
+ -- override the global
+ #print_strict_params on
+ declare
+ x record;
+ p1 int := 2;
+ p3 text := 'foo';
+ begin
+ -- too many rows
+ select * from foo where f1 > p1 or f1::text = p3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+ end$$ language plpgsql;
+
+ select footest();
+
-- test scrollable cursor support
create function sc_test() returns setof integer as $$
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
