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
*** 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(&paramstr);
+       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, &paramtypeid,
+                                               &paramtypmod, &paramdatum, 
&paramisnull);
+ 
+               if (paramno > 1)
+                       appendStringInfo(&paramstr, ", ");
+ 
+               if (paramisnull)
+                       appendStringInfo(&paramstr, "%s = NULL", 
curvar->refname);
+               else
+               {
+                       char *value = convert_value_to_string(estate, 
paramdatum, paramtypeid);
+                       appendStringInfo(&paramstr, "%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(&paramstr);
+       for (i = 0; i < ppd->nargs; ++i)
+       {
+               if (i > 0)
+                       appendStringInfoString(&paramstr, ", ");
+ 
+               if (ppd->nulls[i] == 'n')
+                       appendStringInfo(&paramstr, "$%d = NULL", i+1);
+               else
+               {
+                       char *value = convert_value_to_string(estate, 
ppd->values[i], ppd->types[i]);
+                   appendStringInfo(&paramstr, "$%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 $$
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to