Re: plpgsql raise - parameters can be expressions - Mailing list pgsql-patches

Tom Lane wrote:
> I'd lean to the former myself --- which actually does suggest that this
> patch is not ready for application yet, because it banks on the
> assumption that "x,y,z" should be treated as a single expression.

Attached is a revised patch that stores the RAISE parameters as a list
of expressions, and evaluates that list via repeated application of
exec_eval_expr(). I also updated the regression tests to not remove the
SQLERRM/SQLSTATE tests. Applied to HEAD.

-Neil
Index: doc/src/sgml/plpgsql.sgml
===================================================================
RCS file: /var/lib/cvs/pgsql/doc/src/sgml/plpgsql.sgml,v
retrieving revision 1.71
diff -c -r1.71 plpgsql.sgml
*** doc/src/sgml/plpgsql.sgml    10 Jun 2005 16:23:09 -0000    1.71
--- doc/src/sgml/plpgsql.sgml    14 Jun 2005 04:57:37 -0000
***************
*** 2533,2541 ****
     <para>
      Inside the format string, <literal>%</literal> is replaced by the
      next optional argument's string representation. Write
!     <literal>%%</literal> to emit a literal <literal>%</literal>. Note
!     that the optional arguments must presently be simple variables,
!     not expressions, and the format must be a simple string literal.
     </para>

     <!--
--- 2533,2541 ----
     <para>
      Inside the format string, <literal>%</literal> is replaced by the
      next optional argument's string representation. Write
!     <literal>%%</literal> to emit a literal <literal>%</literal>.
!     Arguments can be simple variables or expressions,
!     and the format must be a simple string literal.
     </para>

     <!--
Index: src/pl/plpgsql/src/gram.y
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.75
diff -c -r1.75 gram.y
*** src/pl/plpgsql/src/gram.y    10 Jun 2005 16:23:11 -0000    1.75
--- src/pl/plpgsql/src/gram.y    14 Jun 2005 06:24:36 -0000
***************
*** 135,142 ****
  %type <exception>    proc_exception
  %type <condition>    proc_conditions

! %type <list>    raise_params
! %type <ival>    raise_level raise_param
  %type <str>        raise_msg

  %type <list>    getdiag_list
--- 135,142 ----
  %type <exception>    proc_exception
  %type <condition>    proc_conditions

!
! %type <ival>    raise_level
  %type <str>        raise_msg

  %type <list>    getdiag_list
***************
*** 1157,1165 ****
                      }
                  ;

! stmt_raise        : K_RAISE lno raise_level raise_msg raise_params ';'
                      {
                          PLpgSQL_stmt_raise        *new;

                          new = palloc(sizeof(PLpgSQL_stmt_raise));

--- 1157,1166 ----
                      }
                  ;

! stmt_raise        : K_RAISE lno raise_level raise_msg
                      {
                          PLpgSQL_stmt_raise        *new;
+                         int    tok;

                          new = palloc(sizeof(PLpgSQL_stmt_raise));

***************
*** 1167,1187 ****
                          new->lineno        = $2;
                          new->elog_level = $3;
                          new->message    = $4;
!                         new->params        = $5;

!                         $$ = (PLpgSQL_stmt *)new;
!                     }
!                 | K_RAISE lno raise_level raise_msg ';'
!                     {
!                         PLpgSQL_stmt_raise        *new;

!                         new = palloc(sizeof(PLpgSQL_stmt_raise));

!                         new->cmd_type    = PLPGSQL_STMT_RAISE;
!                         new->lineno        = $2;
!                         new->elog_level = $3;
!                         new->message    = $4;
!                         new->params        = NIL;

                          $$ = (PLpgSQL_stmt *)new;
                      }
--- 1168,1200 ----
                          new->lineno        = $2;
                          new->elog_level = $3;
                          new->message    = $4;
!                         new->params        = NIL;

!                         tok = yylex();

!                         /*
!                          * We expect either a semi-colon, which
!                          * indicates no parameters, or a comma that
!                          * begins the list of parameter expressions
!                          */
!                         if (tok != ',' && tok != ';')
!                             yyerror("syntax error");

!                         if (tok == ',')
!                         {
!                             PLpgSQL_expr *expr;
!                             int term;
!
!                             for (;;)
!                             {
!                                 expr = read_sql_construct(',', ';', ", or ;",
!                                                           "SELECT ",
!                                                           true, true, &term);
!                                 new->params = lappend(new->params, expr);
!                                 if (term == ';')
!                                     break;
!                             }
!                         }

                          $$ = (PLpgSQL_stmt *)new;
                      }
***************
*** 1219,1240 ****
                      }
                  ;

- raise_params    : raise_params raise_param
-                     {
-                         $$ = lappend_int($1, $2);
-                     }
-                 | raise_param
-                     {
-                         $$ = list_make1_int($1);
-                     }
-                 ;
-
- raise_param        : ',' T_SCALAR
-                     {
-                         $$ = yylval.scalar->dno;
-                     }
-                 ;
-
  loop_body        : proc_sect K_END K_LOOP ';'
                      { $$ = $1; }
                  ;
--- 1232,1237 ----
***************
*** 1658,1664 ****
   * expected:    text to use in complaining that terminator was not found
   * sqlstart:    text to prefix to the accumulated SQL text
   * isexpression: whether to say we're reading an "expression" or a "statement"
!  * valid_sql:   whether to check the syntax of the expression (plus sqlstart)
   * endtoken:    if not NULL, ending token is stored at *endtoken
   *                (this is only interesting if until2 isn't zero)
   */
--- 1655,1661 ----
   * expected:    text to use in complaining that terminator was not found
   * sqlstart:    text to prefix to the accumulated SQL text
   * isexpression: whether to say we're reading an "expression" or a "statement"
!  * valid_sql:   whether to check the syntax of the expr (prefixed with sqlstart)
   * endtoken:    if not NULL, ending token is stored at *endtoken
   *                (this is only interesting if until2 isn't zero)
   */
Index: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.143
diff -c -r1.143 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c    10 Jun 2005 16:23:11 -0000    1.143
--- src/pl/plpgsql/src/pl_exec.c    14 Jun 2005 06:25:50 -0000
***************
*** 594,600 ****
      error_context_stack = plerrcontext.previous;

      /*
!      * Return the triggers result
       */
      return rettup;
  }
--- 594,600 ----
      error_context_stack = plerrcontext.previous;

      /*
!      * Return the trigger's result
       */
      return rettup;
  }
***************
*** 1095,1116 ****
  exec_stmt_perform(PLpgSQL_execstate *estate, PLpgSQL_stmt_perform *stmt)
  {
      PLpgSQL_expr *expr = stmt->expr;
-     int            rc;
-
-     /*
-      * If not already done create a plan for this expression
-      */
-     if (expr->plan == NULL)
-         exec_prepare_plan(estate, expr);
-
-     rc = exec_run_select(estate, expr, 0, NULL);
-     if (rc != SPI_OK_SELECT)
-         ereport(ERROR,
-                 (errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                errmsg("query \"%s\" did not return data", expr->query)));

      exec_set_found(estate, (estate->eval_processed != 0));
-
      exec_eval_cleanup(estate);

      return PLPGSQL_RC_OK;
--- 1095,1103 ----
  exec_stmt_perform(PLpgSQL_execstate *estate, PLpgSQL_stmt_perform *stmt)
  {
      PLpgSQL_expr *expr = stmt->expr;

+     (void) exec_run_select(estate, expr, 0, NULL);
      exec_set_found(estate, (estate->eval_processed != 0));
      exec_eval_cleanup(estate);

      return PLPGSQL_RC_OK;
***************
*** 1941,1955 ****
                          (errcode(ERRCODE_SYNTAX_ERROR),
                           errmsg("too few parameters specified for RAISE")));

!             exec_eval_datum(estate, estate->datums[lfirst_int(current_param)],
!                             InvalidOid,
!                             ¶mtypeid, ¶mvalue, ¶misnull);
              if (paramisnull)
                  extval = "<NULL>";
              else
                  extval = convert_value_to_string(paramvalue, paramtypeid);
              plpgsql_dstring_append(&ds, extval);
              current_param = lnext(current_param);
              continue;
          }

--- 1928,1945 ----
                          (errcode(ERRCODE_SYNTAX_ERROR),
                           errmsg("too few parameters specified for RAISE")));

!             paramvalue = exec_eval_expr(estate,
!                                         (PLpgSQL_expr *) lfirst(current_param),
!                                         ¶misnull,
!                                         ¶mtypeid);
!
              if (paramisnull)
                  extval = "<NULL>";
              else
                  extval = convert_value_to_string(paramvalue, paramtypeid);
              plpgsql_dstring_append(&ds, extval);
              current_param = lnext(current_param);
+             exec_eval_cleanup(estate);
              continue;
          }

Index: src/pl/plpgsql/src/pl_funcs.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/pl_funcs.c,v
retrieving revision 1.42
diff -c -r1.42 pl_funcs.c
*** src/pl/plpgsql/src/pl_funcs.c    14 Jun 2005 00:10:02 -0000    1.42
--- src/pl/plpgsql/src/pl_funcs.c    14 Jun 2005 06:23:23 -0000
***************
*** 885,897 ****
  static void
  dump_raise(PLpgSQL_stmt_raise *stmt)
  {
!     ListCell *l;

      dump_ind();
!     printf("RAISE '%s'", stmt->message);
!     foreach (l, stmt->params)
!         printf(" %d", lfirst_int(l));
!     printf("\n");
  }

  static void
--- 885,904 ----
  static void
  dump_raise(PLpgSQL_stmt_raise *stmt)
  {
!     ListCell *lc;
!     int i = 0;

      dump_ind();
!     printf("RAISE '%s'\n", stmt->message);
!     dump_indent += 2;
!     foreach (lc, stmt->params)
!     {
!         dump_ind();
!         printf("    parameter %d: ", i++);
!         dump_expr((PLpgSQL_expr *) lfirst(lc));
!         printf("\n");
!     }
!     dump_indent -= 2;
  }

  static void
***************
*** 916,922 ****
      {
          dump_ind();
          printf("    target = %d %s\n", stmt->rec->recno, stmt->rec->refname);
!     } else if (stmt->row != NULL)
      {
          dump_ind();
          printf("    target = %d %s\n", stmt->row->rowno, stmt->row->refname);
--- 923,930 ----
      {
          dump_ind();
          printf("    target = %d %s\n", stmt->rec->recno, stmt->rec->refname);
!     }
!     else if (stmt->row != NULL)
      {
          dump_ind();
          printf("    target = %d %s\n", stmt->row->rowno, stmt->row->refname);
Index: src/pl/plpgsql/src/plpgsql.h
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/plpgsql.h,v
retrieving revision 1.62
diff -c -r1.62 plpgsql.h
*** src/pl/plpgsql/src/plpgsql.h    10 Jun 2005 16:23:11 -0000    1.62
--- src/pl/plpgsql/src/plpgsql.h    14 Jun 2005 06:33:24 -0000
***************
*** 515,521 ****
      int            lineno;
      int            elog_level;
      char       *message;
!     List       *params;
  } PLpgSQL_stmt_raise;


--- 515,521 ----
      int            lineno;
      int            elog_level;
      char       *message;
!     List       *params;            /* list of expressions */
  } PLpgSQL_stmt_raise;


Index: src/test/regress/expected/plpgsql.out
===================================================================
RCS file: /var/lib/cvs/pgsql/src/test/regress/expected/plpgsql.out,v
retrieving revision 1.33
diff -c -r1.33 plpgsql.out
*** src/test/regress/expected/plpgsql.out    10 Jun 2005 16:23:11 -0000    1.33
--- src/test/regress/expected/plpgsql.out    14 Jun 2005 06:30:31 -0000
***************
*** 2418,2434 ****
  --
  -- SQLSTATE and SQLERRM test
  --
! -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
! -- blocks
! create function excpt_test() returns void as $$
  begin
      raise notice '% %', sqlstate, sqlerrm;
  end; $$ language plpgsql;
! ERROR:  syntax error at or near "sqlstate" at character 79
! LINE 3:     raise notice '% %', sqlstate, sqlerrm;
!                                 ^
! -- should fail
! create function excpt_test() returns void as $$
  begin
      begin
          begin
--- 2418,2434 ----
  --
  -- SQLSTATE and SQLERRM test
  --
! create function excpt_test1() returns void as $$
  begin
      raise notice '% %', sqlstate, sqlerrm;
  end; $$ language plpgsql;
! -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
! -- blocks
! select excpt_test1();
! ERROR:  column "sqlstate" does not exist
! CONTEXT:  SQL statement "SELECT  sqlstate"
! PL/pgSQL function "excpt_test1" line 2 at raise
! create function excpt_test2() returns void as $$
  begin
      begin
          begin
***************
*** 2436,2445 ****
          end;
      end;
  end; $$ language plpgsql;
! ERROR:  syntax error at or near "sqlstate" at character 108
! LINE 5:          raise notice '% %', sqlstate, sqlerrm;
!                                      ^
! create function excpt_test() returns void as $$
  begin
      begin
          raise exception 'user exception';
--- 2436,2447 ----
          end;
      end;
  end; $$ language plpgsql;
! -- should fail
! select excpt_test2();
! ERROR:  column "sqlstate" does not exist
! CONTEXT:  SQL statement "SELECT  sqlstate"
! PL/pgSQL function "excpt_test2" line 4 at raise
! create function excpt_test3() returns void as $$
  begin
      begin
          raise exception 'user exception';
***************
*** 2458,2471 ****
          raise notice '% %', sqlstate, sqlerrm;
      end;
  end; $$ language plpgsql;
! select excpt_test();
  NOTICE:  caught exception P0001 user exception
  NOTICE:  P0001 user exception
  NOTICE:  caught exception 22012 division by zero
  NOTICE:  P0001 user exception
!  excpt_test
! ------------

  (1 row)

! drop function excpt_test();
--- 2460,2493 ----
          raise notice '% %', sqlstate, sqlerrm;
      end;
  end; $$ language plpgsql;
! select excpt_test3();
  NOTICE:  caught exception P0001 user exception
  NOTICE:  P0001 user exception
  NOTICE:  caught exception 22012 division by zero
  NOTICE:  P0001 user exception
!  excpt_test3
! -------------
!
! (1 row)
!
! drop function excpt_test1();
! drop function excpt_test2();
! drop function excpt_test3();
! -- parameters of raise stmt can be expressions
! create function raise_exprs() returns void as $$
! declare
!     a integer[] = '{10,20,30}';
!     c varchar = 'xyz';
!     i integer;
! begin
!     i := 2;
!     raise notice '%; %; %; %; %; %', a, a[i], c, (select c || 'abc'), row(10,'aaa',NULL,30), NULL;
! end;$$ language plpgsql;
! select raise_exprs();
! NOTICE:  {10,20,30}; 20; xyz; xyzabc; (10,aaa,,30); <NULL>
!  raise_exprs
! -------------

  (1 row)

! drop function raise_exprs();
Index: src/test/regress/sql/plpgsql.sql
===================================================================
RCS file: /var/lib/cvs/pgsql/src/test/regress/sql/plpgsql.sql,v
retrieving revision 1.28
diff -c -r1.28 plpgsql.sql
*** src/test/regress/sql/plpgsql.sql    10 Jun 2005 16:23:11 -0000    1.28
--- src/test/regress/sql/plpgsql.sql    14 Jun 2005 06:26:52 -0000
***************
*** 2055,2069 ****
  -- SQLSTATE and SQLERRM test
  --

! -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
! -- blocks
! create function excpt_test() returns void as $$
  begin
      raise notice '% %', sqlstate, sqlerrm;
  end; $$ language plpgsql;

! -- should fail
! create function excpt_test() returns void as $$
  begin
      begin
          begin
--- 2055,2069 ----
  -- SQLSTATE and SQLERRM test
  --

! create function excpt_test1() returns void as $$
  begin
      raise notice '% %', sqlstate, sqlerrm;
  end; $$ language plpgsql;
+ -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
+ -- blocks
+ select excpt_test1();

! create function excpt_test2() returns void as $$
  begin
      begin
          begin
***************
*** 2071,2078 ****
          end;
      end;
  end; $$ language plpgsql;

! create function excpt_test() returns void as $$
  begin
      begin
          raise exception 'user exception';
--- 2071,2080 ----
          end;
      end;
  end; $$ language plpgsql;
+ -- should fail
+ select excpt_test2();

! create function excpt_test3() returns void as $$
  begin
      begin
          raise exception 'user exception';
***************
*** 2092,2096 ****
      end;
  end; $$ language plpgsql;

! select excpt_test();
! drop function excpt_test();
--- 2094,2114 ----
      end;
  end; $$ language plpgsql;

! select excpt_test3();
! drop function excpt_test1();
! drop function excpt_test2();
! drop function excpt_test3();
!
! -- parameters of raise stmt can be expressions
! create function raise_exprs() returns void as $$
! declare
!     a integer[] = '{10,20,30}';
!     c varchar = 'xyz';
!     i integer;
! begin
!     i := 2;
!     raise notice '%; %; %; %; %; %', a, a[i], c, (select c || 'abc'), row(10,'aaa',NULL,30), NULL;
! end;$$ language plpgsql;
!
! select raise_exprs();
! drop function raise_exprs();