Use correct text domain for errcontext() appearing within ereport().

The mechanism added in commit dbdf9679d7d61b03a3bf73af9b095831b7010eb5
for associating the correct translation domain with errcontext strings
potentially fails in cases where errcontext() is used within an ereport()
macro.  Such usage was not originally envisioned for errcontext(), but we
do have a few places that do it.  In this situation, the intended comma
expression becomes just a couple of arguments to errfinish(), which the
compiler might choose to evaluate right-to-left.

Fortunately, in such cases the textdomain for the errcontext string must
be the same as for the surrounding ereport.  So we can fix this by letting
errstart initialize context_domain along with domain; then it will have
the correct value no matter which order the calls occur in.  (Note that
error stack callback functions are not invoked until errfinish, so normal
usage of errcontext won't affect what happens for errcontext calls within
the ereport macro.)

In passing, make sure that errcontext calls within the main backend set
context_domain to something non-NULL.  This isn't a live bug because
NULL would select the current textdomain() setting which should be the
right thing anyway --- but it seems better to handle this completely
consistently with the regular domain field.

Per report from Dmitry Voronin.  Backpatch to 9.3; before that, there
wasn't any attempt to ensure that errcontext strings were translated
in an appropriate domain.

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 13395e38e75c221e117762975613ef05963aa651..bee2c92301e52122682943c6c545e533c847df21 100644 (file)
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -378,6 +378,8 @@ errstart(int elevel, const char *filename, int lineno,
        edata->funcname = funcname;
        /* the default text domain is the backend's */
        edata->domain = domain ? domain : PG_TEXTDOMAIN("postgres");
+       /* initialize context_domain the same way (see set_errcontext_domain()) */
+       edata->context_domain = edata->domain;
        /* Select default errcode based on elevel */
        if (elevel >= ERROR)
                edata->sqlerrcode = ERRCODE_INTERNAL_ERROR;
@@ -728,7 +730,7 @@ errcode_for_socket_access(void)
                char               *fmtbuf; \
                StringInfoData  buf; \
                /* Internationalize the error format string */ \
-               if (translateit && !in_error_recursion_trouble()) \
+               if ((translateit) && !in_error_recursion_trouble()) \
                        fmt = dgettext((domain), fmt);                            \
                /* Expand %m in format string */ \
                fmtbuf = expand_fmt_string(fmt, edata); \
@@ -1048,6 +1050,16 @@ errcontext_msg(const char *fmt,...)
  * translate it.  Instead, each errcontext_msg() call should be preceded by
  * a set_errcontext_domain() call to specify the domain.  This is usually
  * done transparently by the errcontext() macro.
+ *
+ * Although errcontext is primarily meant for use at call sites distant from
+ * the original ereport call, there are a few places that invoke errcontext
+ * within ereport.  The expansion of errcontext as a comma expression calling
+ * set_errcontext_domain then errcontext_msg is problematic in this case,
+ * because the intended comma expression becomes two arguments to errfinish,
+ * which the compiler is at liberty to evaluate in either order.  But in
+ * such a case, the set_errcontext_domain calls must be selecting the same
+ * TEXTDOMAIN value that the errstart call did, so order does not matter
+ * so long as errstart initializes context_domain along with domain.
  */
 int
 set_errcontext_domain(const char *domain)
@@ -1057,7 +1069,8 @@ set_errcontext_domain(const char *domain)
        /* we don't bother incrementing recursion_depth */
        CHECK_STACK_DEPTH();
 
-       edata->context_domain = domain;
+       /* the default text domain is the backend's */
+       edata->context_domain = domain ? domain : PG_TEXTDOMAIN("postgres");
 
        return 0;                                       /* return value does not matter */
 }