On 15.02.2012 20:13, Tom Lane wrote:
Heikki Linnakangas<heikki.linnakan...@enterprisedb.com> writes:
To fix this, we need to somehow pass the caller's text domain to
errcontext(). The most straightforward way is to pass it as an extra
argument. Ideally, errcontext() would be a macro that passes TEXTDOMAIN
to the underlying function, so that you don't need to change all the
callers of errcontext():
#define errcontext(...) errcontext_domain(TEXTDOMAIN, ...)
But that doesn't work, because it would require varags macros. Anyone
know a trick to make that work?
This is pretty ugly, but AFAICS it should work:
#define errcontext set_errcontext_domain(TEXTDOMAIN), real_errcontext
But it might be better in the long run to bite the bullet and make
people change all their errcontext calls. There aren't that many,
especially not outside the core code.
Agreed, and not many of the external modules are translated, anyway.
I played with a few different approaches:
1. Add a variant of errcontext() that takes a text domain argument, so
that the calls look like:
errcontext_domain(TEXTDOMAIN, "PL/Perl anonymous code block");
Straightforward, but it looks silly to have to pass TEXTDOMAIN as an
explicit argument. It's not usually explicitly used in C code at all.
2. Add a new field to ErrorContextCallback struct, indicating the
domain. So to set up a callback, you'd do:
ErrorContextCallback pl_error_context;
/* Set up a callback for error reporting */
pl_error_context.callback = plperl_inline_callback;
pl_error_context.previous = error_context_stack;
pl_error_context.arg = (Datum) 0;
pl_error_context.domain = TEXTDOMAIN;
error_context_stack = &pl_error_context;
A variant of this is to encapsulate that boilerplate code to a new macro
or function, so that you'd do just:
push_error_context(&pl_err_context, plperl_inline_callback, (Datum) 0);
push_error_context macro would automatically set the domain to
TEXTDOMAIN, similar to what ereport() does.
A problem with this approach is that if we add a new field to the
struct, any existing code would leave it uninitialized. That could
easily lead to mysterious crashes.
3. In the callback function, call a new function to set the domain to be
used for the errcontext() calls in that callback:
/* use the right domain to translate the errcontext() calls */
set_errtextdomain();
errcontext("PL/Perl anonymous code block");
Attached is a patch using this approach, I like it the most. Existing
code still works, as far as it works today, and it's easy to add that
set_errtextdomain() call to fix callbacks that have translated message.
Barring objections, I'll commit this.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 0c301b2..e267ff2 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9913,6 +9913,9 @@ rm_redo_error_callback(void *arg)
XLogRecord *record = (XLogRecord *) arg;
StringInfoData buf;
+ /* use the right domain to translate the errcontext() calls */
+ set_errtextdomain();
+
initStringInfo(&buf);
RmgrTable[record->xl_rmid].rm_desc(&buf,
record->xl_info,
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 1fffe1c..59ff1c9 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -914,6 +914,9 @@ sql_function_parse_error_callback(void *arg)
{
parse_error_callback_arg *callback_arg = (parse_error_callback_arg *) arg;
+ /* use the right domain to translate the errcontext() calls */
+ set_errtextdomain();
+
/* See if it's a syntax error; if so, transpose to CREATE FUNCTION */
if (!function_parse_error_transpose(callback_arg->prosrc))
{
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 95fec8d..8533914 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1743,6 +1743,9 @@ CopyFromErrorCallback(void *arg)
{
CopyState cstate = (CopyState) arg;
+ /* use the right domain to translate the errcontext() calls */
+ set_errtextdomain();
+
if (cstate->binary)
{
/* can't usefully display the data */
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index ae8d374..7643733 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -1294,6 +1294,9 @@ sql_exec_error_callback(void *arg)
if (fcache == NULL || fcache->fname == NULL)
return;
+ /* use the right domain to translate the errcontext() calls */
+ set_errtextdomain();
+
/*
* If there is a syntax error position, convert to internal syntax error
*/
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 5e4ae42..fd43106 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2154,7 +2154,10 @@ _SPI_error_callback(void *arg)
internalerrquery(query);
}
else
+ {
+ set_errtextdomain();
errcontext("SQL statement \"%s\"", query);
+ }
}
/*
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 344ebb7..c747bb6 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -4350,6 +4350,9 @@ sql_inline_error_callback(void *arg)
inline_error_callback_arg *callback_arg = (inline_error_callback_arg *) arg;
int syntaxerrposition;
+ /* use the right domain to translate the errcontext() calls */
+ set_errtextdomain();
+
/* If it's a syntax error, convert to internal syntax error report */
syntaxerrposition = geterrposition();
if (syntaxerrposition > 0)
diff --git a/src/backend/parser/parse_type.c b/src/backend/parser/parse_type.c
index 5314954..dcbc275 100644
--- a/src/backend/parser/parse_type.c
+++ b/src/backend/parser/parse_type.c
@@ -644,6 +644,9 @@ pts_error_callback(void *arg)
{
const char *str = (const char *) arg;
+ /* use the right domain to translate the errcontext() calls */
+ set_errtextdomain();
+
errcontext("invalid type name \"%s\"", str);
/*
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 613d754..6630755 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -2849,6 +2849,9 @@ shared_buffer_write_error_callback(void *arg)
{
volatile BufferDesc *bufHdr = (volatile BufferDesc *) arg;
+ /* use the right domain to translate the errcontext() calls */
+ set_errtextdomain();
+
/* Buffer is pinned, so we can read the tag without locking the spinlock */
if (bufHdr != NULL)
{
@@ -2868,6 +2871,9 @@ local_buffer_write_error_callback(void *arg)
{
volatile BufferDesc *bufHdr = (volatile BufferDesc *) arg;
+ /* use the right domain to translate the errcontext() calls */
+ set_errtextdomain();
+
if (bufHdr != NULL)
{
char *path = relpathbackend(bufHdr->tag.rnode, MyBackendId,
diff --git a/src/backend/tsearch/ts_locale.c b/src/backend/tsearch/ts_locale.c
index 5b42a93..b2511bf 100644
--- a/src/backend/tsearch/ts_locale.c
+++ b/src/backend/tsearch/ts_locale.c
@@ -166,6 +166,9 @@ tsearch_readline_callback(void *arg)
{
tsearch_readline_state *stp = (tsearch_readline_state *) arg;
+ /* use the right domain to translate the errcontext() calls */
+ set_errtextdomain();
+
/*
* We can't include the text of the config line for errors that occur
* during t_readline() itself. This is only partly a consequence of our
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 239ac19..3d17036 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -395,6 +395,7 @@ errfinish(int dummy,...)
int elevel = edata->elevel;
MemoryContext oldcontext;
ErrorContextCallback *econtext;
+ const char *olddomain;
recursion_depth++;
CHECK_STACK_DEPTH();
@@ -410,10 +411,23 @@ errfinish(int dummy,...)
* functions will be treated as recursive errors --- this ensures we will
* avoid infinite recursion (see errstart).
*/
+ olddomain = edata->domain;
for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext->previous)
+ {
+ /*
+ * Callback can set the text domain, so restore it after the call.
+ * This isn't strictly necessary, every callback should set the domain
+ * explicitly if the messages are translatable to begin with. But
+ * just in case a callback forgets to do it (perhaps because it's a
+ * an external module that was written before version 9.2, where
+ * errtextdomain() was added), let's be consistent and always use the
+ * domain specified in errstart.
+ */
(*econtext->callback) (econtext->arg);
+ edata->domain = olddomain;
+ }
/*
* If ERROR (not more nor less) we pass it off to the current handler.
@@ -980,6 +994,34 @@ errcontext(const char *fmt,...)
/*
+ * errtextdomain --- set the message domain used for errcontext calls
+ *
+ * This is used in error callback functions, before calling errcontext(). It
+ * sets the message domain that will be used to translate the messages in
+ * the errcontext() call(s) that follow. Normally we set the domain in
+ * errstart(), but an error context callback often resides in a different
+ * module than where the error originates and where errstart() called, so the
+ * text domain needs to be set explicitly in the callback.
+ *
+ * Actually, this affects not only errcontext() calls but also errmsg(),
+ * errdetail() etc, but errcontext() is the only function that is normally
+ * called at a distance from the original ereport() site.
+ */
+int
+errtextdomain(const char *domain)
+{
+ ErrorData *edata = &errordata[errordata_stack_depth];
+
+ /* we don't bother incrementing recursion_depth */
+ CHECK_STACK_DEPTH();
+
+ edata->domain = domain;
+
+ return 0; /* return value does not matter */
+}
+
+
+/*
* errhidestmt --- optionally suppress STATEMENT: field of log entry
*
* This should be called if the message text already includes the statement.
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 7b5bcfa..a78b949 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -178,6 +178,8 @@ errcontext(const char *fmt,...)
the supplied arguments. */
__attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2)));
+extern int errtextdomain(const char *domain);
+
extern int errhidestmt(bool hide_stmt);
extern int errfunction(const char *funcname);
@@ -227,6 +229,15 @@ typedef struct ErrorContextCallback
extern PGDLLIMPORT ErrorContextCallback *error_context_stack;
+/*
+ * This is a convenience macro to set text-domain to the default for the
+ * current source file. This should be called in every error callback
+ * function, before calling errcontext(), to make sure the right gettext
+ * domain is used to translate the context message.
+ */
+#define set_errtextdomain() \
+ errtextdomain(TEXTDOMAIN)
+
/*----------
* API for catching ereport(ERROR) exits. Use these macros like so:
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 7a92f3d..22006e3 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -3649,7 +3649,10 @@ plperl_exec_callback(void *arg)
char *procname = (char *) arg;
if (procname)
+ {
+ set_errtextdomain();
errcontext("PL/Perl function \"%s\"", procname);
+ }
}
/*
@@ -3661,7 +3664,10 @@ plperl_compile_callback(void *arg)
char *procname = (char *) arg;
if (procname)
+ {
+ set_errtextdomain();
errcontext("compilation of PL/Perl function \"%s\"", procname);
+ }
}
/*
@@ -3670,6 +3676,7 @@ plperl_compile_callback(void *arg)
static void
plperl_inline_callback(void *arg)
{
+ set_errtextdomain();
errcontext("PL/Perl anonymous code block");
}
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index d43b8e0..8e53f04 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -908,8 +908,11 @@ plpgsql_compile_error_callback(void *arg)
}
if (plpgsql_error_funcname)
+ {
+ set_errtextdomain();
errcontext("compilation of PL/pgSQL function \"%s\" near line %d",
plpgsql_error_funcname, plpgsql_latest_lineno());
+ }
}
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index a385b9a..a5b9866 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -786,6 +786,9 @@ plpgsql_exec_error_callback(void *arg)
if (estate->err_text == raise_skip_msg)
return;
+ /* use the right domain to translate the errcontext() calls */
+ set_errtextdomain();
+
if (estate->err_text != NULL)
{
/*
diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c
index 280d3ed..0356d56 100644
--- a/src/pl/plpython/plpy_exec.c
+++ b/src/pl/plpython/plpy_exec.c
@@ -458,7 +458,10 @@ plpython_return_error_callback(void *arg)
PLyExecutionContext *exec_ctx = PLy_current_execution_context();
if (exec_ctx->curr_proc)
+ {
+ set_errtextdomain();
errcontext("while creating return value");
+ }
}
static PyObject *
@@ -786,7 +789,10 @@ plpython_trigger_error_callback(void *arg)
PLyExecutionContext *exec_ctx = PLy_current_execution_context();
if (exec_ctx->curr_proc)
+ {
+ set_errtextdomain();
errcontext("while modifying trigger row");
+ }
}
/* execute Python code, propagate Python errors to the backend */
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index c126db9..da7c8ed 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -343,13 +343,17 @@ plpython_error_callback(void *arg)
PLyExecutionContext *exec_ctx = PLy_current_execution_context();
if (exec_ctx->curr_proc)
+ {
+ set_errtextdomain();
errcontext("PL/Python function \"%s\"",
PLy_procedure_name(exec_ctx->curr_proc));
+ }
}
static void
plpython_inline_error_callback(void *arg)
{
+ set_errtextdomain();
errcontext("PL/Python anonymous code block");
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
