Re: [PATCHES] final light versions of Oracle compatibility (SQLSTATE,

2005-06-10 Thread Neil Conway

Tom Lane wrote:

A nonterminal that is not intended to represent any real input, ever,
is just plain weird.


If you say so... PL/PgSQL already uses such a beast, though: the lno 
nonterminal, for example.



Not at all.  The right way to do this, I think, is for the mid-rule
action to palloc the PLpgSQL_exception_block, fill the variables into
that, and return the block as its semantic value.  The end-of-rule
action then picks up the block and adds what it needs to.


Ah, I see -- that makes sense. Attached is a revised patch -- applied to 
HEAD.


-Neil
Index: doc/src/sgml/plpgsql.sgml
===
RCS file: /Users/neilc/local/cvs/pgsql/doc/src/sgml/plpgsql.sgml,v
retrieving revision 1.70
diff -c -r1.70 plpgsql.sgml
*** doc/src/sgml/plpgsql.sgml	7 Jun 2005 02:47:15 -	1.70
--- doc/src/sgml/plpgsql.sgml	10 Jun 2005 15:34:04 -
***
*** 2110,2115 
--- 2110,2126 
don't use EXCEPTION without need.
   
  
+ 
+ 
+  Within an exception handler, the SQLSTATE
+  variable contains the error code that corresponds to the
+  exception that was raised (refer to  for a list of possible error
+  codes). The SQLERRM variable contains the
+  error message associated with the exception. These variables are
+  undefined outside exception handlers.
+ 
+ 
  
  Exceptions with UPDATE/INSERT
  
Index: src/backend/utils/error/elog.c
===
RCS file: /Users/neilc/local/cvs/pgsql/src/backend/utils/error/elog.c,v
retrieving revision 1.159
diff -c -r1.159 elog.c
*** src/backend/utils/error/elog.c	9 Jun 2005 22:29:52 -	1.159
--- src/backend/utils/error/elog.c	10 Jun 2005 15:34:04 -
***
*** 1482,1487 
--- 1482,1507 
  	}
  }
  
+ /*
+  * Unpack MAKE_SQLSTATE code. Note that this returns a pointer to a
+  * static buffer.
+  */
+ char *
+ unpack_sql_state(int sql_state)
+ {
+ 	static char	buf[12];
+ 	int			i;
+ 
+ 	for (i = 0; i < 5; i++)
+ 	{
+ 		buf[i] = PGUNSIXBIT(sql_state);
+ 		sql_state >>= 6;
+ 	}
+ 
+ 	buf[i] = '\0';
+ 	return buf;
+ }
+ 
  
  /*
   * Write error report to server's log
***
*** 1497,1517 
  	appendStringInfo(&buf, "%s:  ", error_severity(edata->elevel));
  
  	if (Log_error_verbosity >= PGERROR_VERBOSE)
! 	{
! 		/* unpack MAKE_SQLSTATE code */
! 		char		tbuf[12];
! 		int			ssval;
! 		int			i;
! 
! 		ssval = edata->sqlerrcode;
! 		for (i = 0; i < 5; i++)
! 		{
! 			tbuf[i] = PGUNSIXBIT(ssval);
! 			ssval >>= 6;
! 		}
! 		tbuf[i] = '\0';
! 		appendStringInfo(&buf, "%s: ", tbuf);
! 	}
  
  	if (edata->message)
  		append_with_tabs(&buf, edata->message);
--- 1517,1523 
  	appendStringInfo(&buf, "%s:  ", error_severity(edata->elevel));
  
  	if (Log_error_verbosity >= PGERROR_VERBOSE)
! 		appendStringInfo(&buf, "%s: ", unpack_sql_state(edata->sqlerrcode));
  
  	if (edata->message)
  		append_with_tabs(&buf, edata->message);
Index: src/include/utils/elog.h
===
RCS file: /Users/neilc/local/cvs/pgsql/src/include/utils/elog.h,v
retrieving revision 1.78
diff -c -r1.78 elog.h
*** src/include/utils/elog.h	31 Dec 2004 22:03:46 -	1.78
--- src/include/utils/elog.h	10 Jun 2005 15:34:04 -
***
*** 282,287 
--- 282,288 
  
  /* Other exported functions */
  extern void DebugFileOpen(void);
+ extern char *unpack_sql_state(int sql_state);
  
  /*
   * Write errors to stderr (or by equal means when stderr is
Index: src/pl/plpgsql/src/gram.y
===
RCS file: /Users/neilc/local/cvs/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.74
diff -c -r1.74 gram.y
*** src/pl/plpgsql/src/gram.y	8 Jun 2005 00:49:36 -	1.74
--- src/pl/plpgsql/src/gram.y	10 Jun 2005 16:02:48 -
***
*** 92,97 
--- 92,98 
  		PLpgSQL_stmt_block		*program;
  		PLpgSQL_condition		*condition;
  		PLpgSQL_exception		*exception;
+ 		PLpgSQL_exception_block	*exception_block;
  		PLpgSQL_nsitem			*nsitem;
  		PLpgSQL_diag_item		*diagitem;
  }
***
*** 129,135 
  %type 	stmt_dynexecute stmt_getdiag
  %type 	stmt_open stmt_fetch stmt_close stmt_null
  
! %type 	exception_sect proc_exceptions
  %type 	proc_exception
  %type 	proc_conditions
  
--- 130,137 
  %type 	stmt_dynexecute stmt_getdiag
  %type 	stmt_open stmt_fetch stmt_close stmt_null
  
! %type 	proc_exceptions
! %type  exception_sect
  %type 	proc_exception
  %type 	proc_conditions
  
***
*** 1495,1503 
  ;
  
  exception_sect	:
! 	{ $$ = NIL; }
! | K_EXCEPTION proc_exceptions
! 	{ $$ = $2; }
  ;
  
  proc_exceptions	: proc_exceptions proc_exception
--- 1497,1534 
  ;
  
  exception_sect	:
! 	{ $$ = NULL; }
! | K_EXCEPTION lno
! 	{
! 		/*
! 		 * We use a mid-rule action to add these
! 		 * special varia

Re: [PATCHES] final light versions of Oracle compatibility (SQLSTATE, GREATEST,

2005-06-09 Thread Tom Lane
Neil Conway <[EMAIL PROTECTED]> writes:
> Considering that the Bison manual suggests that it implements 
> mid-rule actions by introducing an implicit bogus non-terminal ([1]),

Indeed ... and the reason that they bothered to do that is that mid-rule
actions are more understandable ;-).  A nonterminal that is not intended
to represent any real input, ever, is just plain weird.

> Unless we want two contiguous mid-rule actions 
> (which is even _less_ readable), we'll need to futz with adding another 
> member to %union to hold the two varnos the mid-rule action will 
> produce.

Not at all.  The right way to do this, I think, is for the mid-rule
action to palloc the PLpgSQL_exception_block, fill the variables into
that, and return the block as its semantic value.  The end-of-rule
action then picks up the block and adds what it needs to.

One reason this is cleaner is that it scales to more SQLERRx variables
without further renumbering of the rule components.

regards, tom lane

---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])


Re: [PATCHES] final light versions of Oracle compatibility (SQLSTATE,

2005-06-09 Thread Neil Conway

Tom Lane wrote:

Right, mid-rule actions were what I had in mind.  They're not uglier
than introducing empty nonterminals


Well, IMHO they make the grammar rather hard to read when the action has 
multiple lines (we would need at least 6 lines of code in the mid-rule 
action, I believe). Unless we want two contiguous mid-rule actions 
(which is even _less_ readable), we'll need to futz with adding another 
member to %union to hold the two varnos the mid-rule action will 
produce. Considering that the Bison manual suggests that it implements 
mid-rule actions by introducing an implicit bogus non-terminal ([1]), I 
don't think there is likely to be a difference in performance either 
way, and I think mid-rule actions don't offer a notational improvement 
in this case.


-Neil

[1] 
http://www.gnu.org/software/bison/manual/html_mono/bison.html#Mid_002dRule-Actions, 
toward the end of the section


---(end of broadcast)---
TIP 8: explain analyze is your friend


Re: [PATCHES] final light versions of Oracle compatibility (SQLSTATE, GREATEST,

2005-06-09 Thread Tom Lane
Neil Conway <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> I still find the grammar changes to be an ugly kluge --- it should be
>> possible to do this without introducing bogus nonterminals.

> The scope-local variables need to be added to the namespace by the time 
> that we parse the WHEN clauses. I can see two ways to do that: adding a 
> bogus non-terminal, or using a mid-rule action. Mid-rule actions are 
> pretty ugly, though. Is there a better alternative?

Right, mid-rule actions were what I had in mind.  They're not uglier
than introducing empty nonterminals --- and in fact plpgsql's grammar
already relies on 'em.  The cursor variable declaration production
(about line 359 in CVS tip) seems to me to offer a very direct parallel
to what we want to do here.

regards, tom lane

---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])


Re: [PATCHES] final light versions of Oracle compatibility (SQLSTATE,

2005-06-09 Thread Neil Conway

Tom Lane wrote:

I still find the grammar changes to be an ugly kluge --- it should be
possible to do this without introducing bogus nonterminals.


The scope-local variables need to be added to the namespace by the time 
that we parse the WHEN clauses. I can see two ways to do that: adding a 
bogus non-terminal, or using a mid-rule action. Mid-rule actions are 
pretty ugly, though. Is there a better alternative?



The ns push/pop operations don't appear to be correctly matched


Sorry, asleep at the switch -- the ns_push/pop stuff isn't even needed, 
it was from an early revision of the patch.


A revised patch is attached -- once the nonterminal stuff is sorted I'll 
apply it.


-Neil
Index: doc/src/sgml/plpgsql.sgml
===
RCS file: /var/lib/cvs/pgsql/doc/src/sgml/plpgsql.sgml,v
retrieving revision 1.70
diff -c -r1.70 plpgsql.sgml
*** doc/src/sgml/plpgsql.sgml	7 Jun 2005 02:47:15 -	1.70
--- doc/src/sgml/plpgsql.sgml	10 Jun 2005 01:22:04 -
***
*** 2110,2115 
--- 2110,2126 
don't use EXCEPTION without need.
   
  
+ 
+ 
+  Within an exception handler, the SQLSTATE
+  variable contains the error code that corresponds to the
+  exception that was raised (refer to  for a list of possible error
+  codes). The SQLERRM variable contains the
+  error message associated with the exception. These variables are
+  undefined outside exception handlers.
+ 
+ 
  
  Exceptions with UPDATE/INSERT
  
Index: src/backend/utils/error/elog.c
===
RCS file: /var/lib/cvs/pgsql/src/backend/utils/error/elog.c,v
retrieving revision 1.159
diff -c -r1.159 elog.c
*** src/backend/utils/error/elog.c	9 Jun 2005 22:29:52 -	1.159
--- src/backend/utils/error/elog.c	10 Jun 2005 01:22:04 -
***
*** 1482,1487 
--- 1482,1507 
  	}
  }
  
+ /*
+  * Unpack MAKE_SQLSTATE code. Note that this returns a pointer to a
+  * static buffer.
+  */
+ char *
+ unpack_sql_state(int sql_state)
+ {
+ 	static char	buf[12];
+ 	int			i;
+ 
+ 	for (i = 0; i < 5; i++)
+ 	{
+ 		buf[i] = PGUNSIXBIT(sql_state);
+ 		sql_state >>= 6;
+ 	}
+ 
+ 	buf[i] = '\0';
+ 	return buf;
+ }
+ 
  
  /*
   * Write error report to server's log
***
*** 1497,1517 
  	appendStringInfo(&buf, "%s:  ", error_severity(edata->elevel));
  
  	if (Log_error_verbosity >= PGERROR_VERBOSE)
! 	{
! 		/* unpack MAKE_SQLSTATE code */
! 		char		tbuf[12];
! 		int			ssval;
! 		int			i;
! 
! 		ssval = edata->sqlerrcode;
! 		for (i = 0; i < 5; i++)
! 		{
! 			tbuf[i] = PGUNSIXBIT(ssval);
! 			ssval >>= 6;
! 		}
! 		tbuf[i] = '\0';
! 		appendStringInfo(&buf, "%s: ", tbuf);
! 	}
  
  	if (edata->message)
  		append_with_tabs(&buf, edata->message);
--- 1517,1523 
  	appendStringInfo(&buf, "%s:  ", error_severity(edata->elevel));
  
  	if (Log_error_verbosity >= PGERROR_VERBOSE)
! 		appendStringInfo(&buf, "%s: ", unpack_sql_state(edata->sqlerrcode));
  
  	if (edata->message)
  		append_with_tabs(&buf, edata->message);
Index: src/include/utils/elog.h
===
RCS file: /var/lib/cvs/pgsql/src/include/utils/elog.h,v
retrieving revision 1.78
diff -c -r1.78 elog.h
*** src/include/utils/elog.h	31 Dec 2004 22:03:46 -	1.78
--- src/include/utils/elog.h	10 Jun 2005 01:22:04 -
***
*** 282,287 
--- 282,288 
  
  /* Other exported functions */
  extern void DebugFileOpen(void);
+ extern char *unpack_sql_state(int sql_state);
  
  /*
   * Write errors to stderr (or by equal means when stderr is
Index: src/pl/plpgsql/src/gram.y
===
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.74
diff -c -r1.74 gram.y
*** src/pl/plpgsql/src/gram.y	8 Jun 2005 00:49:36 -	1.74
--- src/pl/plpgsql/src/gram.y	10 Jun 2005 01:22:04 -
***
*** 92,97 
--- 92,98 
  		PLpgSQL_stmt_block		*program;
  		PLpgSQL_condition		*condition;
  		PLpgSQL_exception		*exception;
+ 		PLpgSQL_exception_block	*exception_block;
  		PLpgSQL_nsitem			*nsitem;
  		PLpgSQL_diag_item		*diagitem;
  }
***
*** 129,137 
  %type 	stmt_dynexecute stmt_getdiag
  %type 	stmt_open stmt_fetch stmt_close stmt_null
  
! %type 	exception_sect proc_exceptions
  %type 	proc_exception
  %type 	proc_conditions
  
  %type 	raise_params
  %type 	raise_level raise_param
--- 130,140 
  %type 	stmt_dynexecute stmt_getdiag
  %type 	stmt_open stmt_fetch stmt_close stmt_null
  
! %type 	proc_exceptions
! %type  exception_sect
  %type 	proc_exception
  %type 	proc_conditions
+ %type 		sqlstate_var sqlerrm_var
  
  %type 	raise_params
  %type 	raise_level raise_param
***
*** 1495,1503 
  ;
  
  exception_sect	:
! 	{ $$ = NIL; }
! | K_EXCEPTION proc_exceptions
! 	{ $$ = $2; }
  ;
  

Re: [PATCHES] final light versions of Oracle compatibility (SQLSTATE, GREATEST,

2005-06-09 Thread Tom Lane
Bruce Momjian  writes:
> Also, do we want these features?  Do they duplicate anything we already
> have?

This patch only covers SQLSTATE/SQLERRM for plpgsql, which I think we're
now agreed on doing.  The other stuff is still open for discussion ---
personally I'm OK with the concept of LEAST and GREATEST, not too
excited about the other things Pavel suggested.  (Note I've not looked
at the proposed code for least/greatest yet, it may have issues.)

regards, tom lane

---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])


Re: [PATCHES] final light versions of Oracle compatibility (SQLSTATE,

2005-06-09 Thread Bruce Momjian

Also, do we want these features?  Do they duplicate anything we already
have?

---

Tom Lane wrote:
> Neil Conway <[EMAIL PROTECTED]> writes:
> > Attached is a revised version of this patch. I'll apply it tonight or 
> > tomorrow, barring any objections.
> 
> I still find the grammar changes to be an ugly kluge --- it should be
> possible to do this without introducing bogus nonterminals.
> 
> The ns push/pop operations don't appear to be correctly matched
> (consider multiple WHEN clauses, a case the regression test does not
> cover), nor do they surround the places where the variables are created.
> It is likely that you don't need a push/pop at all; if it appears to
> work now it's because the end of the block results in a pop and so
> the variables disappear then anyway.
> 
> The patch is sloppy about whether free_var() is static or not.
> 
> I find the proposed change in plpgsql_ns_additem a distinct
> disimprovement --- it's dubious even as a micro-optimization
> and it certainly hurts legibility.
> 
>   regards, tom lane
> 
> ---(end of broadcast)---
> TIP 6: Have you searched our list archives?
> 
>http://archives.postgresql.org
> 

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]


Re: [PATCHES] final light versions of Oracle compatibility (SQLSTATE, GREATEST,

2005-06-09 Thread Tom Lane
Neil Conway <[EMAIL PROTECTED]> writes:
> Attached is a revised version of this patch. I'll apply it tonight or 
> tomorrow, barring any objections.

I still find the grammar changes to be an ugly kluge --- it should be
possible to do this without introducing bogus nonterminals.

The ns push/pop operations don't appear to be correctly matched
(consider multiple WHEN clauses, a case the regression test does not
cover), nor do they surround the places where the variables are created.
It is likely that you don't need a push/pop at all; if it appears to
work now it's because the end of the block results in a pop and so
the variables disappear then anyway.

The patch is sloppy about whether free_var() is static or not.

I find the proposed change in plpgsql_ns_additem a distinct
disimprovement --- it's dubious even as a micro-optimization
and it certainly hurts legibility.

regards, tom lane

---(end of broadcast)---
TIP 6: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] final light versions of Oracle compatibility (SQLSTATE, GREATEST,

2005-06-08 Thread Neil Conway

Pavel Stehule wrote:

1. SQLSTATE and SQLERRM exists only on exception's block, and allways
carry info about some exception.


Attached is a revised version of this patch. I'll apply it tonight or 
tomorrow, barring any objections.


-Neil
Index: doc/src/sgml/plpgsql.sgml
===
RCS file: /var/lib/cvs/pgsql/doc/src/sgml/plpgsql.sgml,v
retrieving revision 1.70
diff -c -r1.70 plpgsql.sgml
*** doc/src/sgml/plpgsql.sgml	7 Jun 2005 02:47:15 -	1.70
--- doc/src/sgml/plpgsql.sgml	9 Jun 2005 03:26:28 -
***
*** 2110,2115 
--- 2110,2126 
don't use EXCEPTION without need.
   
  
+ 
+ 
+  Within an exception handler, the SQLSTATE
+  variable contains the error code that corresponds to the
+  exception that was raised (refer to  for a list of possible error
+  codes). The SQLERRM variable contains the
+  error message associated with the exception. These variables are
+  undefined outside exception handlers.
+ 
+ 
  
  Exceptions with UPDATE/INSERT
  
Index: src/backend/utils/error/elog.c
===
RCS file: /var/lib/cvs/pgsql/src/backend/utils/error/elog.c,v
retrieving revision 1.158
diff -c -r1.158 elog.c
*** src/backend/utils/error/elog.c	12 Mar 2005 01:54:44 -	1.158
--- src/backend/utils/error/elog.c	8 Jun 2005 04:59:14 -
***
*** 1451,1456 
--- 1451,1476 
  	}
  }
  
+ /*
+  * Unpack MAKE_SQLSTATE code. Note that this returns a pointer to a
+  * static buffer.
+  */
+ char *
+ unpack_sql_state(int sql_state)
+ {
+ 	static char	buf[12];
+ 	int			i;
+ 
+ 	for (i = 0; i < 5; i++)
+ 	{
+ 		buf[i] = PGUNSIXBIT(sql_state);
+ 		sql_state >>= 6;
+ 	}
+ 
+ 	buf[i] = '\0';
+ 	return buf;
+ }
+ 
  
  /*
   * Write error report to server's log
***
*** 1466,1486 
  	appendStringInfo(&buf, "%s:  ", error_severity(edata->elevel));
  
  	if (Log_error_verbosity >= PGERROR_VERBOSE)
! 	{
! 		/* unpack MAKE_SQLSTATE code */
! 		char		tbuf[12];
! 		int			ssval;
! 		int			i;
! 
! 		ssval = edata->sqlerrcode;
! 		for (i = 0; i < 5; i++)
! 		{
! 			tbuf[i] = PGUNSIXBIT(ssval);
! 			ssval >>= 6;
! 		}
! 		tbuf[i] = '\0';
! 		appendStringInfo(&buf, "%s: ", tbuf);
! 	}
  
  	if (edata->message)
  		append_with_tabs(&buf, edata->message);
--- 1486,1492 
  	appendStringInfo(&buf, "%s:  ", error_severity(edata->elevel));
  
  	if (Log_error_verbosity >= PGERROR_VERBOSE)
! 		appendStringInfo(&buf, "%s: ", unpack_sql_state(edata->sqlerrcode));
  
  	if (edata->message)
  		append_with_tabs(&buf, edata->message);
Index: src/include/utils/elog.h
===
RCS file: /var/lib/cvs/pgsql/src/include/utils/elog.h,v
retrieving revision 1.78
diff -c -r1.78 elog.h
*** src/include/utils/elog.h	31 Dec 2004 22:03:46 -	1.78
--- src/include/utils/elog.h	8 Jun 2005 04:59:58 -
***
*** 282,287 
--- 282,288 
  
  /* Other exported functions */
  extern void DebugFileOpen(void);
+ extern char *unpack_sql_state(int sql_state);
  
  /*
   * Write errors to stderr (or by equal means when stderr is
Index: src/pl/plpgsql/src/gram.y
===
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.74
diff -c -r1.74 gram.y
*** src/pl/plpgsql/src/gram.y	8 Jun 2005 00:49:36 -	1.74
--- src/pl/plpgsql/src/gram.y	9 Jun 2005 02:13:21 -
***
*** 92,97 
--- 92,98 
  		PLpgSQL_stmt_block		*program;
  		PLpgSQL_condition		*condition;
  		PLpgSQL_exception		*exception;
+ 		PLpgSQL_exception_block	*exception_block;
  		PLpgSQL_nsitem			*nsitem;
  		PLpgSQL_diag_item		*diagitem;
  }
***
*** 129,137 
  %type 	stmt_dynexecute stmt_getdiag
  %type 	stmt_open stmt_fetch stmt_close stmt_null
  
! %type 	exception_sect proc_exceptions
  %type 	proc_exception
  %type 	proc_conditions
  
  %type 	raise_params
  %type 	raise_level raise_param
--- 130,140 
  %type 	stmt_dynexecute stmt_getdiag
  %type 	stmt_open stmt_fetch stmt_close stmt_null
  
! %type 	proc_exceptions
! %type  exception_sect
  %type 	proc_exception
  %type 	proc_conditions
+ %type 		sqlstate_var sqlerrm_var
  
  %type 	raise_params
  %type 	raise_level raise_param
***
*** 1495,1519 
  ;
  
  exception_sect	:
! 	{ $$ = NIL; }
! | K_EXCEPTION proc_exceptions
! 	{ $$ = $2; }
  ;
  
  proc_exceptions	: proc_exceptions proc_exception
  		{
  			$$ = lappend($1, $2);
  		}
  | proc_exception
  		{
  			$$ = list_make1($1);
  		}
  ;
  
  proc_exception	: K_WHEN lno proc_conditions K_THEN proc_sect
  	{
  		PLpgSQL_exception *new;
  
  		new = palloc0(sizeof(PLpgSQL_exception));
  		new->lineno = $2;
  		new->conditions = $3;
--- 1498,1554 
  ;
  
  exception_sect	:
!