Re: Different gettext domain needed for error context - Mailing list pgsql-hackers

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