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

tglsfdc · tglsfdc · commit 19f326619696 · 2015-01-12T12:40:31.000-05:00
The mechanism added in commit dbdf967 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
@@ -377,6 +377,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;
@@ -721,7 +723,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); \
@@ -1019,6 +1021,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)
@@ -1028,7 +1040,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 */
 }
