Hi
2015-04-24 19:16 GMT+02:00 Joel Jacobson <j...@trustly.com>:
> On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule <pavel.steh...@gmail.com>
> wrote:
> >> Example:
> >>
> >> context_messages = -warning, -error, +notice
> >
> >
> > I prefer your first proposal - and there is a precedent for plpgsql -
> > plpgsql_extra_checks
> >
> > It is clean for anybody. +-identifiers looks like horrible httpd config.
> :)
>
> I have to agree on that :) Just thought this is the best we can do if
> we want to reduce the number of GUCs to a minimum.
>
I played with some prototype and I am thinking so we need only one GUC
plpgsql.display_context_messages = 'none'; -- compatible with current
plpgsql.display_context_messages = 'all';
plpgsql.display_context_messages = 'exception, log'; -- what I prefer
I implemented [ (WITH|WITHOUT) CONTEXT ] clause for RAISE statement
RAISE NOTICE WITH CONTEXT 'some message';
RAISE NOTICE WITH CONTEXT USING message = 'some message';
RAISE EXCEPTION WITHOUT CONTEXT 'other message';
The patch is very small with full functionality (without documentation) - I
am thinking so it can work. This patch is back compatible - and allow to
change default behave simply.
plpgsql.display_context_messages can be simplified to some like
plpgsql.display_context_min_messages
What do you think about it?
Regards
Pavel
commit cf9e23a29162ac55fcab1ac4d9e7a24492de0736
Author: Pavel Stehule <pavel.steh...@gooddata.com>
Date: Sat Apr 25 22:09:28 2015 +0200
initial implementation of (WITH|WITHOUT) CONTEXT clause to plpgsql RAISE statement.
initial implementation of plpgsql GUC plpgsql.display_context_messages
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index deefb1f..ea0dac5 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2921,6 +2921,7 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
char *err_table = NULL;
char *err_schema = NULL;
ListCell *lc;
+ bool hide_ctx;
/* RAISE with no parameters: re-throw current exception */
if (stmt->condname == NULL && stmt->message == NULL &&
@@ -3080,10 +3081,42 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
err_message = pstrdup(unpack_sql_state(err_code));
}
- /*
- * Throw the error (may or may not come back)
- */
- estate->err_text = raise_skip_msg; /* suppress traceback of raise */
+ if (stmt->context_info == PLPGSQL_CONTEXT_DISPLAY)
+ hide_ctx = false;
+ else if (stmt->context_info == PLPGSQL_CONTEXT_SUPPRESS)
+ {
+ hide_ctx = true;
+ }
+ else
+ {
+ /* we display a messages via plpgsql_display_context_messages */
+ switch (stmt->elog_level)
+ {
+ case ERROR:
+ hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_ERROR);
+ break;
+ case WARNING:
+ hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_WARNING);
+ break;
+ case NOTICE:
+ hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_NOTICE);
+ break;
+ case INFO:
+ hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_INFO);
+ break;
+ case LOG:
+ hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_LOG);
+ break;
+ case DEBUG1:
+ hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_DEBUG);
+ break;
+ default:
+ elog(ERROR, "unexpected RAISE statement level");
+ }
+ }
+
+ if (hide_ctx)
+ estate->err_text = raise_skip_msg;
ereport(stmt->elog_level,
(err_code ? errcode(err_code) : 0,
@@ -3099,7 +3132,8 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
(err_table != NULL) ?
err_generic_string(PG_DIAG_TABLE_NAME, err_table) : 0,
(err_schema != NULL) ?
- err_generic_string(PG_DIAG_SCHEMA_NAME, err_schema) : 0));
+ err_generic_string(PG_DIAG_SCHEMA_NAME, err_schema) : 0,
+ errhidecontext(hide_ctx)));
estate->err_text = NULL; /* un-suppress... */
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 4026e41..48914a7 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -259,6 +259,7 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt);
%token <keyword> K_CONSTANT
%token <keyword> K_CONSTRAINT
%token <keyword> K_CONSTRAINT_NAME
+%token <keyword> K_CONTEXT
%token <keyword> K_CONTINUE
%token <keyword> K_CURRENT
%token <keyword> K_CURSOR
@@ -341,6 +342,8 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt);
%token <keyword> K_WARNING
%token <keyword> K_WHEN
%token <keyword> K_WHILE
+%token <keyword> K_WITH
+%token <keyword> K_WITHOUT
%%
@@ -1716,6 +1719,7 @@ stmt_raise : K_RAISE
new->cmd_type = PLPGSQL_STMT_RAISE;
new->lineno = plpgsql_location_to_lineno(@1);
new->elog_level = ERROR; /* default */
+ new->context_info = PLPGSQL_CONTEXT_DEFAULT;
new->condname = NULL;
new->message = NULL;
new->params = NIL;
@@ -1773,6 +1777,21 @@ stmt_raise : K_RAISE
if (tok == 0)
yyerror("unexpected end of function definition");
+ /* Optional choose about including context */
+ if (tok == K_WITH || tok == K_WITHOUT)
+ {
+ if (tok == K_WITH)
+ new->context_info = PLPGSQL_CONTEXT_DISPLAY;
+ else
+ new->context_info = PLPGSQL_CONTEXT_SUPPRESS;
+
+ /* keyword CONTEXT is required */
+ if (yylex() != K_CONTEXT)
+ yyerror("expected CONTEXT");
+
+ tok = yylex();
+ }
+
/*
* Next we can have a condition name, or
* equivalently SQLSTATE 'xxxxx', or a string
@@ -2350,6 +2369,7 @@ unreserved_keyword :
| K_CONSTANT
| K_CONSTRAINT
| K_CONSTRAINT_NAME
+ | K_CONTEXT
| K_CONTINUE
| K_CURRENT
| K_CURSOR
@@ -2412,6 +2432,8 @@ unreserved_keyword :
| K_USE_VARIABLE
| K_VARIABLE_CONFLICT
| K_WARNING
+ | K_WITH
+ | K_WITHOUT
;
%%
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index 266c314..c47365b 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -30,6 +30,9 @@ static bool plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSo
static void plpgsql_extra_warnings_assign_hook(const char *newvalue, void *extra);
static void plpgsql_extra_errors_assign_hook(const char *newvalue, void *extra);
+static bool plpgsql_display_context_messages_check_hook(char **newvalue, void **extra, GucSource source);
+static void plpgsql_display_context_messages_assign_hook(const char *newvalue, void *extra);
+
PG_MODULE_MAGIC;
/* Custom GUC variable */
@@ -51,9 +54,12 @@ char *plpgsql_extra_errors_string = NULL;
int plpgsql_extra_warnings;
int plpgsql_extra_errors;
+char *plpgsql_display_context_messages_string = NULL;
+
/* Hook for plugins */
PLpgSQL_plugin **plugin_ptr = NULL;
+int plpgsql_display_context_messages;
static bool
plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource source)
@@ -128,6 +134,87 @@ plpgsql_extra_errors_assign_hook(const char *newvalue, void *extra)
plpgsql_extra_errors = *((int *) extra);
}
+static bool
+plpgsql_display_context_messages_check_hook(char **newvalue, void **extra, GucSource source)
+{
+ int display_context_messages = 0;
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+ int *myextra;
+
+ if (pg_strcasecmp(*newvalue, "all") == 0)
+ {
+ display_context_messages = PLPGSQL_DISPLAY_CONTEXT_ALL;
+ }
+ else if (pg_strcasecmp(*newvalue, "none") == 0)
+ {
+ display_context_messages = PLPGSQL_DISPLAY_CONTEXT_NONE;
+ }
+ else
+ {
+ /* Need a modifiable copy of string */
+ rawstring = pstrdup(*newvalue);
+
+ /* Parse string into list of identifiers */
+ if (!SplitIdentifierString(rawstring, ',', &elemlist))
+ {
+ /* syntax error in list */
+ GUC_check_errdetail("List syntax is invalid.");
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ foreach(l, elemlist)
+ {
+ char *tok = (char *) lfirst(l);
+
+ if (pg_strcasecmp(tok, "exception") == 0)
+ display_context_messages |= PLPGSQL_DISPLAY_CONTEXT_ERROR;
+ else if (pg_strcasecmp(tok, "warning") == 0)
+ display_context_messages |= PLPGSQL_DISPLAY_CONTEXT_WARNING;
+ else if (pg_strcasecmp(tok, "notice") == 0)
+ display_context_messages |= PLPGSQL_DISPLAY_CONTEXT_NOTICE;
+ else if (pg_strcasecmp(tok, "info") == 0)
+ display_context_messages |= PLPGSQL_DISPLAY_CONTEXT_INFO;
+ else if (pg_strcasecmp(tok, "log") == 0)
+ display_context_messages |= PLPGSQL_DISPLAY_CONTEXT_LOG;
+ else if (pg_strcasecmp(tok, "debug") == 0)
+ display_context_messages |= PLPGSQL_DISPLAY_CONTEXT_DEBUG;
+ else if (pg_strcasecmp(tok, "all") == 0 || pg_strcasecmp(tok, "none") == 0)
+ {
+ GUC_check_errdetail("Key word \"%s\" cannot be combined with other key words.", tok);
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+ else
+ {
+ GUC_check_errdetail("Unrecognized key word: \"%s\".", tok);
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+ }
+
+ pfree(rawstring);
+ list_free(elemlist);
+ }
+
+ myextra = (int *) malloc(sizeof(int));
+ *myextra = display_context_messages;
+ *extra = (void *) myextra;
+
+ return true;
+
+}
+
+static void plpgsql_display_context_messages_assign_hook(const char *newvalue, void *extra)
+{
+ plpgsql_display_context_messages = *((int *) extra);
+}
+
/*
* _PG_init() - library load-time initialization
@@ -190,6 +277,16 @@ _PG_init(void)
plpgsql_extra_errors_assign_hook,
NULL);
+ DefineCustomStringVariable("plpgsql.display_context_messages",
+ gettext_noop(""),
+ NULL,
+ &plpgsql_display_context_messages_string,
+ "none",
+ PGC_USERSET, GUC_LIST_INPUT,
+ plpgsql_display_context_messages_check_hook,
+ plpgsql_display_context_messages_assign_hook,
+ NULL);
+
EmitWarningsOnPlaceholders("plpgsql");
plpgsql_HashTableInit();
diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
index 683fdab..973cab8 100644
--- a/src/pl/plpgsql/src/pl_scanner.c
+++ b/src/pl/plpgsql/src/pl_scanner.c
@@ -107,6 +107,7 @@ static const ScanKeyword unreserved_keywords[] = {
PG_KEYWORD("constant", K_CONSTANT, UNRESERVED_KEYWORD)
PG_KEYWORD("constraint", K_CONSTRAINT, UNRESERVED_KEYWORD)
PG_KEYWORD("constraint_name", K_CONSTRAINT_NAME, UNRESERVED_KEYWORD)
+ PG_KEYWORD("context", K_CONTEXT, UNRESERVED_KEYWORD)
PG_KEYWORD("continue", K_CONTINUE, UNRESERVED_KEYWORD)
PG_KEYWORD("current", K_CURRENT, UNRESERVED_KEYWORD)
PG_KEYWORD("cursor", K_CURSOR, UNRESERVED_KEYWORD)
@@ -170,6 +171,8 @@ static const ScanKeyword unreserved_keywords[] = {
PG_KEYWORD("use_variable", K_USE_VARIABLE, UNRESERVED_KEYWORD)
PG_KEYWORD("variable_conflict", K_VARIABLE_CONFLICT, UNRESERVED_KEYWORD)
PG_KEYWORD("warning", K_WARNING, UNRESERVED_KEYWORD)
+ PG_KEYWORD("with", K_WITH, UNRESERVED_KEYWORD)
+ PG_KEYWORD("without", K_WITHOUT, UNRESERVED_KEYWORD)
};
static const int num_unreserved_keywords = lengthof(unreserved_keywords);
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index bec773a..cac3833 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -168,6 +168,18 @@ typedef enum
} PLpgSQL_resolve_option;
+/* --------
+ * Manipulation with context of exception
+ * --------
+ */
+enum
+{
+ PLPGSQL_CONTEXT_DISPLAY,
+ PLPGSQL_CONTEXT_SUPPRESS,
+ PLPGSQL_CONTEXT_DEFAULT
+};
+
+
/**********************************************************************
* Node and structure definitions
**********************************************************************/
@@ -619,6 +631,7 @@ typedef struct
int cmd_type;
int lineno;
int elog_level;
+ int context_info;
char *condname; /* condition name, SQLSTATE, or NULL */
char *message; /* old-style message format literal, or NULL */
List *params; /* list of expressions for old-style message */
@@ -922,6 +935,18 @@ extern MemoryContext compile_tmp_cxt;
extern PLpgSQL_plugin **plugin_ptr;
+/* display context messages */
+#define PLPGSQL_DISPLAY_CONTEXT_NONE 0
+#define PLPGSQL_DISPLAY_CONTEXT_ALL ((int) ~0)
+#define PLPGSQL_DISPLAY_CONTEXT_ERROR 2
+#define PLPGSQL_DISPLAY_CONTEXT_WARNING 4
+#define PLPGSQL_DISPLAY_CONTEXT_NOTICE 8
+#define PLPGSQL_DISPLAY_CONTEXT_INFO 16
+#define PLPGSQL_DISPLAY_CONTEXT_LOG 32
+#define PLPGSQL_DISPLAY_CONTEXT_DEBUG 64
+
+extern int plpgsql_display_context_messages;
+
/**********************************************************************
* Function declarations
**********************************************************************/
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 78e5a85..6e75185 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -5426,3 +5426,38 @@ end;
$$;
ERROR: unhandled assertion
CONTEXT: PL/pgSQL function inline_code_block line 3 at ASSERT
+--possibility to enforce context message displaying
+do $$
+begin
+ raise notice with context 'hello';
+end;
+$$;
+NOTICE: hello
+CONTEXT: PL/pgSQL function inline_code_block line 3 at RAISE
+do $$
+begin
+ raise exception with context 'hello';
+end;
+$$;
+ERROR: hello
+CONTEXT: PL/pgSQL function inline_code_block line 3 at RAISE
+set plpgsql.display_context_messages = 'notice, exception';
+do $$
+begin
+ raise notice 'some notice';
+ raise exception 'some exception';
+end;
+$$;
+NOTICE: some notice
+CONTEXT: PL/pgSQL function inline_code_block line 3 at RAISE
+ERROR: some exception
+CONTEXT: PL/pgSQL function inline_code_block line 4 at RAISE
+-- possibility to suppress default
+do $$
+begin
+ raise notice without context 'some notice';
+ raise exception without context 'some exception';
+end;
+$$;
+NOTICE: some notice
+ERROR: some exception
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index e19e415..927da16 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -4265,3 +4265,35 @@ exception when others then
null; -- do nothing
end;
$$;
+
+
+--possibility to enforce context message displaying
+do $$
+begin
+ raise notice with context 'hello';
+end;
+$$;
+
+do $$
+begin
+ raise exception with context 'hello';
+end;
+$$;
+
+set plpgsql.display_context_messages = 'notice, exception';
+do $$
+begin
+ raise notice 'some notice';
+ raise exception 'some exception';
+end;
+$$;
+
+-- possibility to suppress default
+do $$
+begin
+ raise notice without context 'some notice';
+ raise exception without context 'some exception';
+end;
+$$;
+
+
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
