Re: [PATCHES] final light versions of Oracle compatibility (SQLSTATE,
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,
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,
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,
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,
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,
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,
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,
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,
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 : !