Re: Different gettext domain needed for error context

On 15.02.2012 20:13, Tom Lane wrote:
> Heikki Linnakangas<heikki.linnakangas@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