Re: pgsql: Minor cleanup for recent SQLSTATE / SQLERRM - Mailing list pgsql-committers

Tom Lane wrote:
> I'm not sure you should have done that; IMHO the right thing is for
> Bruce to back this patch out and ask for a resubmission

I thought about that, but I don't see a need to back out the patch: it
is not fundamentally broken or invasive to the rest of the tree.
Needlessly backing it out and then reapplying an improved version would
seem to only create more CVS traffic.

> you just made it a lot harder for him to do so.

*grumble*, due to CVS being completely broken... Attached is the patch I
applied, so if someone wants to revert both changes they can do so more
easily.

-Neil
Index: src/pl/plpgsql/src/gram.y
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.70
diff -c -r1.70 gram.y
*** src/pl/plpgsql/src/gram.y    26 May 2005 00:16:31 -0000    1.70
--- src/pl/plpgsql/src/gram.y    26 May 2005 02:48:24 -0000
***************
*** 285,291 ****
                                           plpgsql_build_datatype(TEXTOID, -1), true);
                          $$.sqlerrm_varno = var->dno;
                          plpgsql_add_initdatums(NULL);
!                     };

  decl_sect        : opt_label
                      {
--- 285,292 ----
                                           plpgsql_build_datatype(TEXTOID, -1), true);
                          $$.sqlerrm_varno = var->dno;
                          plpgsql_add_initdatums(NULL);
!                     }
!                 ;

  decl_sect        : opt_label
                      {
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.139
diff -c -r1.139 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c    26 May 2005 00:16:31 -0000    1.139
--- src/pl/plpgsql/src/pl_exec.c    26 May 2005 02:51:42 -0000
***************
*** 760,766 ****
        var = (PLpgSQL_var *) (estate->datums[block->sqlerrm_varno]);
         var->isnull = false;
        var->freeval = true;
!       var->value = DirectFunctionCall1(textin, CStringGetDatum("Sucessful completion"));

      /*
       * First initialize all variables declared in this block
--- 760,766 ----
        var = (PLpgSQL_var *) (estate->datums[block->sqlerrm_varno]);
         var->isnull = false;
        var->freeval = true;
!       var->value = DirectFunctionCall1(textin, CStringGetDatum("Successful completion"));

      /*
       * First initialize all variables declared in this block
***************
*** 777,783 ****

                      if (var->freeval)
                      {
!                         pfree((void *) (var->value));
                          var->freeval = false;
                      }

--- 777,783 ----

                      if (var->freeval)
                      {
!                         pfree(DatumGetPointer(var->value));
                          var->freeval = false;
                      }

***************
*** 872,884 ****
              CurrentResourceOwner = oldowner;

              /* set SQLSTATE and SQLERRM variables */
-
              var = (PLpgSQL_var *) (estate->datums[block->sqlstate_varno]);
!             pfree((void *) (var->value));
              var->value = DirectFunctionCall1(textin, CStringGetDatum(unpack_sql_state(edata->sqlerrcode)));
!
              var = (PLpgSQL_var *) (estate->datums[block->sqlerrm_varno]);
!             pfree((void *) (var->value));
              var->value = DirectFunctionCall1(textin, CStringGetDatum(edata->message));

              /*
--- 872,883 ----
              CurrentResourceOwner = oldowner;

              /* set SQLSTATE and SQLERRM variables */
              var = (PLpgSQL_var *) (estate->datums[block->sqlstate_varno]);
!             pfree(DatumGetPointer(var->value));
              var->value = DirectFunctionCall1(textin, CStringGetDatum(unpack_sql_state(edata->sqlerrcode)));
!
              var = (PLpgSQL_var *) (estate->datums[block->sqlerrm_varno]);
!             pfree(DatumGetPointer(var->value));
              var->value = DirectFunctionCall1(textin, CStringGetDatum(edata->message));

              /*
Index: src/test/regress/expected/plpgsql.out
===================================================================
RCS file: /var/lib/cvs/pgsql/src/test/regress/expected/plpgsql.out,v
retrieving revision 1.29
diff -c -r1.29 plpgsql.out
*** src/test/regress/expected/plpgsql.out    26 May 2005 00:16:31 -0000    1.29
--- src/test/regress/expected/plpgsql.out    26 May 2005 03:16:16 -0000
***************
*** 2381,2387 ****
  drop function void_return_expr();
  drop function missing_return_expr();
  -- test SQLSTATE and SQLERRM
! create or replace function trap_exceptions() returns void as $_$
  begin
     begin
       raise exception 'first exception';
--- 2381,2387 ----
  drop function void_return_expr();
  drop function missing_return_expr();
  -- test SQLSTATE and SQLERRM
! create function trap_exceptions() returns void as $_$
  begin
     begin
       raise exception 'first exception';
***************
*** 2398,2404 ****
  end; $_$ language plpgsql;
  select trap_exceptions();
  NOTICE:  P0001 first exception
! NOTICE:  00000 Sucessful completion
  NOTICE:  P0001 last exception
   trap_exceptions
  -----------------
--- 2398,2404 ----
  end; $_$ language plpgsql;
  select trap_exceptions();
  NOTICE:  P0001 first exception
! NOTICE:  00000 Successful completion
  NOTICE:  P0001 last exception
   trap_exceptions
  -----------------
Index: src/test/regress/sql/plpgsql.sql
===================================================================
RCS file: /var/lib/cvs/pgsql/src/test/regress/sql/plpgsql.sql,v
retrieving revision 1.24
diff -c -r1.24 plpgsql.sql
*** src/test/regress/sql/plpgsql.sql    26 May 2005 00:16:31 -0000    1.24
--- src/test/regress/sql/plpgsql.sql    26 May 2005 03:04:11 -0000
***************
*** 2018,2025 ****

  drop function void_return_expr();
  drop function missing_return_expr();
  -- test SQLSTATE and SQLERRM
! create or replace function trap_exceptions() returns void as $_$
  begin
     begin
       raise exception 'first exception';
--- 2018,2026 ----

  drop function void_return_expr();
  drop function missing_return_expr();
+
  -- test SQLSTATE and SQLERRM
! create function trap_exceptions() returns void as $_$
  begin
     begin
       raise exception 'first exception';