plpgsql.print_strict_params
Hi,
Attached is a patch for optionally printing more information on STRICT
failures in PL/PgSQL:
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
-- 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
This parameter is turned off by default to preserve old behaviour, but
can be extremely useful when debugging code in test environments.
I will add this to the open commitfest, but in the meanwhile, any
feedback is appreciated.
Regards,
Marko Tiikkaja
Attachments:
print_strict_params.patchtext/plain; charset=windows-1252; name=print_strict_params.patchDownload
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 1076,1081 **** END;
--- 1076,1088 ----
always sets <literal>FOUND</literal> to true.
</para>
+ <para>
+ The configuration parameter <literal>plpgsql.print_strict_params</>
+ can be enabled to 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.
+ </para>
+
<para>
For <command>INSERT</>/<command>UPDATE</>/<command>DELETE</> with
<literal>RETURNING</>, <application>PL/pgSQL</application> reports
*** 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 (plpgsql_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 (plpgsql_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 (plpgsql_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 (plpgsql_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_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/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 873,878 **** extern IdentifierLookup plpgsql_IdentifierLookup;
--- 873,880 ----
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,3184 ----
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
+ reset plpgsql.print_strict_params;
-- 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,2664 ----
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();
+
+ reset plpgsql.print_strict_params;
+
-- test scrollable cursor support
create function sc_test() returns setof integer as $$
Marko Tiikkaja wrote:
set plpgsql.print_strict_params to true;
Hmm, shouldn't this be a compile option? See the "comp_option"
production in pl_gram.y for existing examples.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2013-09-14 00:39, Alvaro Herrera wrote:
set plpgsql.print_strict_params to true;
Hmm, shouldn't this be a compile option? See the "comp_option"
production in pl_gram.y for existing examples.
I thought about that, but it seemed more like a runtime option to me.
Any particular reason you think it would be better as a compile time option?
Regards,
Marko Tiikkaja
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Marko Tiikkaja wrote:
On 2013-09-14 00:39, Alvaro Herrera wrote:
set plpgsql.print_strict_params to true;
Hmm, shouldn't this be a compile option? See the "comp_option"
production in pl_gram.y for existing examples.I thought about that, but it seemed more like a runtime option to
me. Any particular reason you think it would be better as a compile
time option?
Seems like something that would be useful to change per function, rather
than globally, no?
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2013-09-14 03:33, Alvaro Herrera wrote:
Marko Tiikkaja wrote:
I thought about that, but it seemed more like a runtime option to
me. Any particular reason you think it would be better as a compile
time option?Seems like something that would be useful to change per function, rather
than globally, no?
The way I see it, STRICT is like an assertion, and you *would* pretty
much always change it globally.
However, it would be useful to also have the option to set it for a
single function only. Will look into that. Thanks for the suggestion!
Regards,
Marko Tiikkaja
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
Attachments:
print_strict_params_v2.patchtext/plain; charset=windows-1252; name=print_strict_params_v2.patchDownload
*** 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 $$
2013/9/15 Marko Tiikkaja <marko@joh.to>:
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.
I'm taking a look at this patch as part of the current commitfest [*],
(It's the first time I've "formally" reviewed a patch for a commitfest
so please let me know if I'm missing something.)
[*] https://commitfest.postgresql.org/action/patch_view?id=1221
Submission review
-----------------
* Is the patch in a patch format which has context?
Yes.
* Does it apply cleanly to the current git master?
Yes. No new files are introduced by this patch.
* Does it include reasonable tests, necessary doc patches, etc?
Yes.
However the sample function provided in the documentation throws a
runtime error due to a missing FROM-clause entry.
Usability review
----------------
* Does the patch actually implement that?
Yes.
* Do we want that?
It seems like it would be useful; no opposition has been raised on
-hackers so far.
* Do we already have it?
No.
* Does it follow SQL spec, or the community-agreed behavior?
SQL spec: n/a. I do note that it makes use of the "#" syntax
before the body of a PL/pgSQL function, which is currently only
used for "#variable_conflict" [*]. I can imagine this syntax might
be used for other purposes down the road, so it might be worth
keeping an eye on it before it becomes a hodge-podge of ad-hoc
options.
[*] http://www.postgresql.org/docs/current/static/plpgsql-implementation.html
* Does it include pg_dump support (if applicable)?
n/a
* Are there dangers?
I don't see any.
* Have all the bases been covered?
This lineis in "exec_get_query_params()" and "exec_get_dynquery_params()":
return "(no parameters)";
Presumably the message will escape translation and this line should be better
written as:
return _("(no parameters)");
Also, if the offending query parameter contains a single quote, the output
is not escaped:
postgres=# select get_userid(E'foo''');
ERROR: query returned more than one row
DETAIL: p1 = 'foo''
CONTEXT: PL/pgSQL function get_userid(text) line 9 at SQL statement
Not sure if that's a real problem though.
Feature test
------------
* Does the feature work as advertised?
Yes.
* Are there corner cases the author has failed to consider?
I can't see any.
* Are there any assertion failures or crashes?
No.
Performance review
------------------
* Does the patch slow down simple tests?
No.
* If it claims to improve performance, does it?
n/a
* Does it slow down other things?
No.
Coding review
-------------
* Does it follow the project coding guidelines?
Yes.
The functions added in pl_exec.c - "exec_get_query_params()" and
"exec_get_dynquery_params()" do strike me as potentially misnamed,
as they don't actually execute anything but are more utility
functions for generating formatted output.
Maybe "format_query_params()" etc. would be better?
* Are there portability issues?
I don't think so.
* Will it work on Windows/BSD etc?
Tested on OS X and Linux. I don't see anything, e.g. system calls,
which might stop it working on Windows.
* Are the comments sufficient and accurate?
"exec_get_query_params()" and "exec_get_dynquery_params()"
could do with some brief comments explaining their purpose.
* Does it do what it says, correctly?
As far as I can tell, yes.
* Does it produce compiler warnings?
No.
* Can you make it crash?
So far, no.
Architecture review
-------------------
* Is everything done in a way that fits together coherently with other
features/modules?
Patch affects PL/pgSQL only.
* Are there interdependencies that can cause problems?
No.
Regards
Ian Barwick
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 9/16/13 8:04 AM, Ian Lawrence Barwick wrote:
I'm taking a look at this patch as part of the current commitfest [*],
Thanks!
However the sample function provided in the documentation throws a
runtime error due to a missing FROM-clause entry.
Ugh. I'll look into fixing that.
* Does it follow SQL spec, or the community-agreed behavior?
SQL spec: n/a. I do note that it makes use of the "#" syntax
before the body of a PL/pgSQL function, which is currently only
used for "#variable_conflict" [*]. I can imagine this syntax might
be used for other purposes down the road, so it might be worth
keeping an eye on it before it becomes a hodge-podge of ad-hoc
options.
Agreed. I have two patches like this on the commitfest and one more
cooking, so if more than one of these make it into PG, we should
probably discuss how the general mechanism should work and look like in
the future.
* Have all the bases been covered?
This lineis in "exec_get_query_params()" and "exec_get_dynquery_params()":
return "(no parameters)";
Presumably the message will escape translation and this line should be better
written as:
return _("(no parameters)");
Nice catch. Will look into this. Another option would be to simply
omit the DETAIL line if there were no parameters. At this very moment
I'm thinking that might be a bit nicer.
Also, if the offending query parameter contains a single quote, the output
is not escaped:postgres=# select get_userid(E'foo''');
ERROR: query returned more than one row
DETAIL: p1 = 'foo''
CONTEXT: PL/pgSQL function get_userid(text) line 9 at SQL statementNot sure if that's a real problem though.
Hmm.. I should probably look at what happens when the parameters to a
prepared statement are currently logged and imitate that.
* Does it follow the project coding guidelines?
Yes.The functions added in pl_exec.c - "exec_get_query_params()" and
"exec_get_dynquery_params()" do strike me as potentially misnamed,
as they don't actually execute anything but are more utility
functions for generating formatted output.Maybe "format_query_params()" etc. would be better?
Agreed, they could use some renaming.
* Are the comments sufficient and accurate?
"exec_get_query_params()" and "exec_get_dynquery_params()"
could do with some brief comments explaining their purpose.
Agreed.
(It's the first time I've "formally" reviewed a patch for a commitfest
so please let me know if I'm missing something.)
I personally think you did an excellent job. Huge thanks so far!
Regards,
Marko Tiikkaja
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi,
Attached is a patch with the following changes:
On 16/09/2013 10:57, I wrote:
On 9/16/13 8:04 AM, Ian Lawrence Barwick wrote:
However the sample function provided in the documentation throws a
runtime error due to a missing FROM-clause entry.Ugh. I'll look into fixing that.
Fixed.
This lineis in "exec_get_query_params()" and "exec_get_dynquery_params()":
return "(no parameters)";
Presumably the message will escape translation and this line should be better
written as:
return _("(no parameters)");Nice catch. Will look into this. Another option would be to simply
omit the DETAIL line if there were no parameters. At this very moment
I'm thinking that might be a bit nicer.
Decided to remove the DETAIL line in these cases.
Also, if the offending query parameter contains a single quote, the output
is not escaped:postgres=# select get_userid(E'foo''');
ERROR: query returned more than one row
DETAIL: p1 = 'foo''
CONTEXT: PL/pgSQL function get_userid(text) line 9 at SQL statementNot sure if that's a real problem though.
Hmm.. I should probably look at what happens when the parameters to a
prepared statement are currently logged and imitate that.
Yup, they're escaped. Did just that. Also copied the "parameters: "
prefix on the DETAIL line from there.
The functions added in pl_exec.c - "exec_get_query_params()" and
"exec_get_dynquery_params()" do strike me as potentially misnamed,
as they don't actually execute anything but are more utility
functions for generating formatted output.Maybe "format_query_params()" etc. would be better?
Agreed, they could use some renaming.
I went with format_expr_params and format_preparedparamsdata.
* Are the comments sufficient and accurate?
"exec_get_query_params()" and "exec_get_dynquery_params()"
could do with some brief comments explaining their purpose.Agreed.
Still agreeing with both of us, added a comment each.
I also added the new keywords to the unreserved_keywords production, as
per the instructions near the beginning of pl_gram.y.
Regards,
Marko Tiikkaja
Attachments:
print_strict_params_v3.patchtext/plain; charset=windows-1252; name=print_strict_params_v3.patchDownload
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 1077,1082 **** END;
--- 1077,1106 ----
</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
+ FROM users 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
***************
*** 221,226 **** static Portal exec_dynquery_with_params(PLpgSQL_execstate *estate,
--- 221,231 ----
PLpgSQL_expr *dynquery, List *params,
const char *portalname, int cursorOptions);
+ static char *format_expr_params(PLpgSQL_execstate *estate,
+ const PLpgSQL_expr *expr);
+ static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
+ const PreparedParamsData *ppd);
+
/* ----------
* plpgsql_exec_function Called by the call handler for
***************
*** 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);
}
--- 3396,3435 ----
if (n == 0)
{
if (stmt->strict)
+ {
+ char *errdetail;
+
+ if (estate->func->print_strict_params)
+ errdetail = format_expr_params(estate, expr);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_NO_DATA_FOUND),
! errmsg("query returned no rows"),
! errdetail ?
! errdetail_internal("parameters: %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 = format_expr_params(estate, expr);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_TOO_MANY_ROWS),
! errmsg("query returned more than one row"),
! errdetail ?
! errdetail_internal("parameters: %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,
--- 3469,3475 ----
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);
--- 3494,3504 ----
***************
*** 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);
}
--- 3590,3630 ----
if (n == 0)
{
if (stmt->strict)
+ {
+ char *errdetail;
+
+ if (estate->func->print_strict_params)
+ errdetail = format_preparedparamsdata(estate, ppd);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_NO_DATA_FOUND),
! errmsg("query returned no rows"),
! errdetail ?
! errdetail_internal("parameters: %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 = format_preparedparamsdata(estate, ppd);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_TOO_MANY_ROWS),
! errmsg("query returned more than one row"),
! errdetail ?
! errdetail_internal("parameters: %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,
--- 3640,3648 ----
*/
}
+ if (ppd)
+ free_params_data(ppd);
+
/* Release any result from SPI_execute, as well as the querystring */
SPI_freetuptable(SPI_tuptable);
pfree(querystr);
***************
*** 6456,6458 **** exec_dynquery_with_params(PLpgSQL_execstate *estate,
--- 6507,6609 ----
return portal;
}
+
+ /*
+ * Return a formatted string with information about an expression's parameters,
+ * or NULL if the expression does not take any parameters.
+ */
+ static char *
+ format_expr_params(PLpgSQL_execstate *estate,
+ const PLpgSQL_expr *expr)
+ {
+ int paramno;
+ int dno;
+ StringInfoData paramstr;
+ Bitmapset *tmpset;
+
+ if (!expr->paramnos)
+ return NULL;
+
+ initStringInfo(¶mstr);
+ tmpset = bms_copy(expr->paramnos);
+ paramno = 0;
+ 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);
+
+ appendStringInfo(¶mstr, "%s%s = ",
+ paramno > 0 ? ", " : "",
+ curvar->refname);
+
+ if (paramisnull)
+ appendStringInfoString(¶mstr, "NULL");
+ else
+ {
+ char *value = convert_value_to_string(estate, paramdatum, paramtypeid);
+ char *p;
+ appendStringInfoCharMacro(¶mstr, '\'');
+ for (p = value; *p; p++)
+ {
+ if (*p == '\'') /* double single quotes */
+ appendStringInfoCharMacro(¶mstr, *p);
+ appendStringInfoCharMacro(¶mstr, *p);
+ }
+ appendStringInfoCharMacro(¶mstr, '\'');
+ }
+
+ paramno++;
+ }
+ bms_free(tmpset);
+
+ return paramstr.data;
+ }
+
+ /*
+ * Return a formatted string with information about PreparedParamsData, or NULL
+ * if the there are no parameters.
+ */
+ static char *
+ format_preparedparamsdata(PLpgSQL_execstate *estate,
+ const PreparedParamsData *ppd)
+ {
+ int paramno;
+ StringInfoData paramstr;
+
+ if (!ppd)
+ return NULL;
+
+ initStringInfo(¶mstr);
+ for (paramno = 0; paramno < ppd->nargs; paramno++)
+ {
+ appendStringInfo(¶mstr, "%s$%d = ",
+ paramno > 0 ? ", " : "",
+ paramno + 1);
+
+ if (ppd->nulls[paramno] == 'n')
+ appendStringInfoString(¶mstr, "NULL");
+ else
+ {
+ char *value = convert_value_to_string(estate, ppd->values[paramno], ppd->types[paramno]);
+ char *p;
+ appendStringInfoCharMacro(¶mstr, '\'');
+ for (p = value; *p; p++)
+ {
+ if (*p == '\'') /* double single quotes */
+ appendStringInfoCharMacro(¶mstr, *p);
+ appendStringInfoCharMacro(¶mstr, *p);
+ }
+ appendStringInfoCharMacro(¶mstr, '\'');
+ }
+ }
+
+ return paramstr.data;
+ }
*** 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 :
| ';'
;
***************
*** 2294,2305 **** unreserved_keyword :
--- 2308,2322 ----
| K_NO
| K_NOTICE
| K_OPTION
+ | K_ON
+ | K_OFF
| K_PG_CONTEXT
| K_PG_DATATYPE_NAME
| K_PG_EXCEPTION_CONTEXT
| K_PG_EXCEPTION_DETAIL
| K_PG_EXCEPTION_HINT
| K_PRIOR
+ | K_PRINT_STRICT_PARAMS
| K_QUERY
| K_RELATIVE
| K_RESULT_OID
*** 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,3213 ----
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: parameters: 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: parameters: 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
+ 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: parameters: $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: parameters: $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
+ 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: parameters: 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 $$
Hi
Sorry for the delay on following up on this.
2013/9/18 Marko Tiikkaja <marko@joh.to>:
Hi,
Attached is a patch with the following changes:
On 16/09/2013 10:57, I wrote:
On 9/16/13 8:04 AM, Ian Lawrence Barwick wrote:
However the sample function provided in the documentation throws a
runtime error due to a missing FROM-clause entry.Ugh. I'll look into fixing that.
Fixed.
Confirmed :)
This lineis in "exec_get_query_params()" and
"exec_get_dynquery_params()":return "(no parameters)";
Presumably the message will escape translation and this line should be
better
written as:
return _("(no parameters)");Nice catch. Will look into this. Another option would be to simply
omit the DETAIL line if there were no parameters. At this very moment
I'm thinking that might be a bit nicer.Decided to remove the DETAIL line in these cases.
Yes, on reflection I think that makes most sense.
Also, if the offending query parameter contains a single quote, the
output
is not escaped:postgres=# select get_userid(E'foo''');
Error: query returned more than one row
DETAIL: p1 = 'foo''
CONTEXT: PL/pgSQL function get_userid(text) line 9 at SQL statementNot sure if that's a real problem though.
Hmm.. I should probably look at what happens when the parameters to a
prepared statement are currently logged and imitate that.Yup, they're escaped. Did just that. Also copied the "parameters: " prefix
on the DETAIL line from there.
The output looks like this now:
postgres=# select get_userid(E'foo''');
ERROR: query returned more than one row
DETAIL: parameters: $1 = 'foo'''
CONTEXT: PL/pgSQL function get_userid(text) line 6 at SQL statement
which looks fine to me.
The functions added in pl_exec.c - "exec_get_query_params()" and
"exec_get_dynquery_params()" do strike me as potentially misnamed,
as they don't actually execute anything but are more utility
functions for generating formatted output.Maybe "format_query_params()" etc. would be better?
Agreed, they could use some renaming.
I went with format_expr_params and format_preparedparamsdata.
Thanks. "format_preparedparamsdata" is a bit hard to scan, but that's
just nitpicking on my part ;)
* Are the comments sufficient and accurate?
"exec_get_query_params()" and "exec_get_dynquery_params()"
could do with some brief comments explaining their purpose.Agreed.
Still agreeing with both of us, added a comment each.
Thanks.
I also added the new keywords to the unreserved_keywords production, as per
the instructions near the beginning of pl_gram.y.
Good catch.
The patch looks good to me now; does the status need to be changed to
"Ready for Committer"?
Regards
Ian Barwick
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2013-09-28 12:31, Ian Lawrence Barwick wrote:
The patch looks good to me now; does the status need to be changed to
"Ready for Committer"?
Yes.
Thanks for reviewing!
Regards,
Marko Tiikkaja
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Sep 28, 2013 at 8:42 AM, Marko Tiikkaja <marko@joh.to> wrote:
On 2013-09-28 12:31, Ian Lawrence Barwick wrote:
The patch looks good to me now; does the status need to be changed to
"Ready for Committer"?Yes.
Thanks for reviewing!
This looks like a nice clean patch. My only concern is that it makes
"on" and "off" unreserved plpgsql keywords. It looks like that will
make them unusable as unquoted identifiers in a few contexts in which
they can now be used. Has there been any discussion about whether
that's OK?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 10/3/13 6:55 PM, Robert Haas wrote:
This looks like a nice clean patch. My only concern is that it makes
"on" and "off" unreserved plpgsql keywords. It looks like that will
make them unusable as unquoted identifiers in a few contexts in which
they can now be used. Has there been any discussion about whether
that's OK?
I don't think there has.
I originally had more ideas for options which you could turn on/off,
which I believe might have justified reserving them, but I'm not sure
any of those will ever happen, at least not as a simple on/off option.
Did you have a better idea for the syntax? The only thing I can think
of is print_strict_params and no_print_strict_params, and I'm not very
fond of that.
Also, in what contexts are unreserved keywords a problem in modern PL/PgSQL?
Regards,
Marko Tiikkaja
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Oct 4, 2013 at 3:53 AM, Marko Tiikkaja <marko@joh.to> wrote:
I don't think there has.
I originally had more ideas for options which you could turn on/off, which I
believe might have justified reserving them, but I'm not sure any of those
will ever happen, at least not as a simple on/off option. Did you have a
better idea for the syntax? The only thing I can think of is
print_strict_params and no_print_strict_params, and I'm not very fond of
that.
Yeah, that doesn't seem good. How about writing the grammar production as
'#' K_PRINT_STRICT_PARAMS option_value
where option_value := T_WORD | unreserved_keyword;
Then you don't need to make ON and OFF keywords; you can just use
strcmp() against the option_value and either throw an error, or set
the flag appropriately.
Also, in what contexts are unreserved keywords a problem in modern PL/PgSQL?
I am not sure I've found all cases where this can matter, but what I
did is flipped through the grammar in less, looking for T_WORD, and
checking for productions where T_WORD was allowed but
unreserved_keyword was not. I found getdiag_target, for_variable,
stmt_execsql, and cursor_variable.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/10/2013 16:08, Robert Haas wrote:
Yeah, that doesn't seem good. How about writing the grammar production as
'#' K_PRINT_STRICT_PARAMS option_value
where option_value := T_WORD | unreserved_keyword;
Then you don't need to make ON and OFF keywords; you can just use
strcmp() against the option_value and either throw an error, or set
the flag appropriately.
Something like the attached?
Regards,
Marko Tiikkaja
Attachments:
print_strict_params_v4.patchtext/plain; charset=windows-1252; name=print_strict_params_v4.patchDownload
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 1077,1082 **** END;
--- 1077,1106 ----
</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
+ FROM users 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
***************
*** 221,226 **** static Portal exec_dynquery_with_params(PLpgSQL_execstate *estate,
--- 221,231 ----
PLpgSQL_expr *dynquery, List *params,
const char *portalname, int cursorOptions);
+ static char *format_expr_params(PLpgSQL_execstate *estate,
+ const PLpgSQL_expr *expr);
+ static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
+ const PreparedParamsData *ppd);
+
/* ----------
* plpgsql_exec_function Called by the call handler for
***************
*** 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);
}
--- 3396,3435 ----
if (n == 0)
{
if (stmt->strict)
+ {
+ char *errdetail;
+
+ if (estate->func->print_strict_params)
+ errdetail = format_expr_params(estate, expr);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_NO_DATA_FOUND),
! errmsg("query returned no rows"),
! errdetail ?
! errdetail_internal("parameters: %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 = format_expr_params(estate, expr);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_TOO_MANY_ROWS),
! errmsg("query returned more than one row"),
! errdetail ?
! errdetail_internal("parameters: %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,
--- 3469,3475 ----
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);
--- 3494,3504 ----
***************
*** 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);
}
--- 3590,3630 ----
if (n == 0)
{
if (stmt->strict)
+ {
+ char *errdetail;
+
+ if (estate->func->print_strict_params)
+ errdetail = format_preparedparamsdata(estate, ppd);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_NO_DATA_FOUND),
! errmsg("query returned no rows"),
! errdetail ?
! errdetail_internal("parameters: %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 = format_preparedparamsdata(estate, ppd);
+ else
+ errdetail = NULL;
+
ereport(ERROR,
(errcode(ERRCODE_TOO_MANY_ROWS),
! errmsg("query returned more than one row"),
! errdetail ?
! errdetail_internal("parameters: %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,
--- 3640,3648 ----
*/
}
+ if (ppd)
+ free_params_data(ppd);
+
/* Release any result from SPI_execute, as well as the querystring */
SPI_freetuptable(SPI_tuptable);
pfree(querystr);
***************
*** 6456,6458 **** exec_dynquery_with_params(PLpgSQL_execstate *estate,
--- 6507,6609 ----
return portal;
}
+
+ /*
+ * Return a formatted string with information about an expression's parameters,
+ * or NULL if the expression does not take any parameters.
+ */
+ static char *
+ format_expr_params(PLpgSQL_execstate *estate,
+ const PLpgSQL_expr *expr)
+ {
+ int paramno;
+ int dno;
+ StringInfoData paramstr;
+ Bitmapset *tmpset;
+
+ if (!expr->paramnos)
+ return NULL;
+
+ initStringInfo(¶mstr);
+ tmpset = bms_copy(expr->paramnos);
+ paramno = 0;
+ 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);
+
+ appendStringInfo(¶mstr, "%s%s = ",
+ paramno > 0 ? ", " : "",
+ curvar->refname);
+
+ if (paramisnull)
+ appendStringInfoString(¶mstr, "NULL");
+ else
+ {
+ char *value = convert_value_to_string(estate, paramdatum, paramtypeid);
+ char *p;
+ appendStringInfoCharMacro(¶mstr, '\'');
+ for (p = value; *p; p++)
+ {
+ if (*p == '\'') /* double single quotes */
+ appendStringInfoCharMacro(¶mstr, *p);
+ appendStringInfoCharMacro(¶mstr, *p);
+ }
+ appendStringInfoCharMacro(¶mstr, '\'');
+ }
+
+ paramno++;
+ }
+ bms_free(tmpset);
+
+ return paramstr.data;
+ }
+
+ /*
+ * Return a formatted string with information about PreparedParamsData, or NULL
+ * if the there are no parameters.
+ */
+ static char *
+ format_preparedparamsdata(PLpgSQL_execstate *estate,
+ const PreparedParamsData *ppd)
+ {
+ int paramno;
+ StringInfoData paramstr;
+
+ if (!ppd)
+ return NULL;
+
+ initStringInfo(¶mstr);
+ for (paramno = 0; paramno < ppd->nargs; paramno++)
+ {
+ appendStringInfo(¶mstr, "%s$%d = ",
+ paramno > 0 ? ", " : "",
+ paramno + 1);
+
+ if (ppd->nulls[paramno] == 'n')
+ appendStringInfoString(¶mstr, "NULL");
+ else
+ {
+ char *value = convert_value_to_string(estate, ppd->values[paramno], ppd->types[paramno]);
+ char *p;
+ appendStringInfoCharMacro(¶mstr, '\'');
+ for (p = value; *p; p++)
+ {
+ if (*p == '\'') /* double single quotes */
+ appendStringInfoCharMacro(¶mstr, *p);
+ appendStringInfoCharMacro(¶mstr, *p);
+ }
+ appendStringInfoCharMacro(¶mstr, '\'');
+ }
+ }
+
+ return paramstr.data;
+ }
*** a/src/pl/plpgsql/src/pl_gram.y
--- b/src/pl/plpgsql/src/pl_gram.y
***************
*** 185,191 **** static List *read_raise_options(void);
%type <forvariable> for_variable
%type <stmt> for_control
! %type <str> any_identifier opt_block_label opt_label
%type <list> proc_sect proc_stmts stmt_elsifs stmt_else
%type <loop_body> loop_body
--- 185,191 ----
%type <forvariable> for_variable
%type <stmt> for_control
! %type <str> any_identifier opt_block_label opt_label option_value
%type <list> proc_sect proc_stmts stmt_elsifs stmt_else
%type <loop_body> loop_body
***************
*** 308,313 **** static List *read_raise_options(void);
--- 308,314 ----
%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
--- 355,369 ----
{
plpgsql_DumpExecTree = true;
}
+ | '#' K_PRINT_STRICT_PARAMS option_value
+ {
+ if (strcmp($3, "on") == 0)
+ plpgsql_curr_compile->print_strict_params = true;
+ else if (strcmp($3, "off") == 0)
+ plpgsql_curr_compile->print_strict_params = false;
+ else
+ elog(ERROR, "unrecognized print_strict_params option %s", $3);
+ }
| '#' K_VARIABLE_CONFLICT K_ERROR
{
plpgsql_curr_compile->resolve_option = PLPGSQL_RESOLVE_ERROR;
***************
*** 368,373 **** comp_option : '#' K_OPTION K_DUMP
--- 378,392 ----
}
;
+ option_value : T_WORD
+ {
+ $$ = $1.ident;
+ }
+ | unreserved_keyword
+ {
+ $$ = pstrdup($1);
+ }
+
opt_semi :
| ';'
;
***************
*** 2300,2305 **** unreserved_keyword :
--- 2319,2325 ----
| K_PG_EXCEPTION_DETAIL
| K_PG_EXCEPTION_HINT
| K_PRIOR
+ | K_PRINT_STRICT_PARAMS
| K_QUERY
| K_RELATIVE
| K_RESULT_OID
*** 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
***************
*** 140,145 **** static const ScanKeyword unreserved_keywords[] = {
--- 140,146 ----
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,3213 ----
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: parameters: 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: parameters: 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
+ 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: parameters: $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: parameters: $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
+ 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: parameters: 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 $$
On Sat, Oct 5, 2013 at 6:15 PM, Marko Tiikkaja <marko@joh.to> wrote:
Something like the attached?
Looks good to me. Committed.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 10/7/13 9:46 PM, Robert Haas wrote:
Looks good to me. Committed.
Thanks.
Also huge thanks to Ian for a phenomenal first review. :-)
Regards,
Marko Tiikkaja
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers