Fix errcontext() function

Started by Chen Huajunabout 13 years ago6 messages
#1Chen Huajun
chenhj@cn.fujitsu.com
2 attachment(s)

Hello,

I am sending patch for errcontext() function.

I use procedural languages to do some operation, but when error occurs
,the CONTEXT error messages from procedural languages doesn't display in
local language.

for example:
--------------------------------------------------------
postgres=# CREATE OR REPLACE FUNCTION logfunc3 (logtxt text) RETURNS
timestamp AS $$
postgres$# BEGIN
postgres$# select * from db;
postgres$# RETURN 'now';
postgres$# END;
postgres$# $$ LANGUAGE plpgsql;
CREATE FUNCTION
postgres=# select logfunc3('test');
ERROR: リレーション"db"は存在しません
行 1: select * from db
QUERY: select * from db
CONTEXT: PL/pgSQL function "logfunc3" line 3 at SQL ステートメント
--------------------------------------------------------
but,“CONTEXT: PL/pgSQL 関数 "logfunc3" の 3 行目の型 SQL ステートメント” is my expected.

There is the same problem in pl/perl and pl/python .

After checking and debuging the source code ,I found the reason.

The reason is that domian setted is wrong. For PL/pgSQL, domain "pgsql" should be setted, but domain setted is "postgres" .

So I considered to fix the bug by updating errcontext() funtion.

The patched portion is at src/include/utils/elog.h and
src/backend/utils/error/elog.c

I invite any ideas how to improve this patch.

Best Regards

Huajun Chen

Attachments:

elog_c.difftext/plain; name=elog_c.diffDownload
*** original/pgsql/src/backend/utils/error/elog.c	2012-11-06 02:11:02.942057121 +0900
--- new/pgsql/src/backend/utils/error/elog.c	2012-11-06 02:11:02.871000898 +0900
***************
*** 673,685 ****
   * to the edata field because the buffer might be considerably larger than
   * really necessary.
   */
! #define EVALUATE_MESSAGE(targetfield, appendval, translateit)  \
  	{ \
  		char		   *fmtbuf; \
  		StringInfoData	buf; \
  		/* Internationalize the error format string */ \
  		if (translateit && !in_error_recursion_trouble()) \
! 			fmt = dgettext(edata->domain, fmt); \
  		/* Expand %m in format string */ \
  		fmtbuf = expand_fmt_string(fmt, edata); \
  		initStringInfo(&buf); \
--- 673,685 ----
   * to the edata field because the buffer might be considerably larger than
   * really necessary.
   */
! #define EVALUATE_MESSAGE_DOMAIN(textdomain, targetfield, appendval, translateit)  \
  	{ \
  		char		   *fmtbuf; \
  		StringInfoData	buf; \
  		/* Internationalize the error format string */ \
  		if (translateit && !in_error_recursion_trouble()) \
! 			fmt = dgettext((textdomain == NULL ? edata->domain : textdomain), fmt); \
  		/* Expand %m in format string */ \
  		fmtbuf = expand_fmt_string(fmt, edata); \
  		initStringInfo(&buf); \
***************
*** 708,713 ****
--- 708,716 ----
  		pfree(buf.data); \
  	}
  
+ #define EVALUATE_MESSAGE(targetfield, appendval, translateit)	\
+ 	EVALUATE_MESSAGE_DOMAIN(NULL, targetfield, appendval, translateit)
+ 
  /*
   * Same as above, except for pluralized error messages.  The calling routine
   * must be declared like "const char *fmt_singular, const char *fmt_plural,
***************
*** 952,958 ****
   * states.
   */
  int
! errcontext(const char *fmt,...)
  {
  	ErrorData  *edata = &errordata[errordata_stack_depth];
  	MemoryContext oldcontext;
--- 955,961 ----
   * states.
   */
  int
! errcontext_domain(const char *domain, const char *fmt,...)
  {
  	ErrorData  *edata = &errordata[errordata_stack_depth];
  	MemoryContext oldcontext;
***************
*** 961,974 ****
  	CHECK_STACK_DEPTH();
  	oldcontext = MemoryContextSwitchTo(ErrorContext);
  
! 	EVALUATE_MESSAGE(context, true, true);
  
  	MemoryContextSwitchTo(oldcontext);
  	recursion_depth--;
  	return 0;					/* return value does not matter */
  }
  
- 
  /*
   * errhidestmt --- optionally suppress STATEMENT: field of log entry
   *
--- 964,976 ----
  	CHECK_STACK_DEPTH();
  	oldcontext = MemoryContextSwitchTo(ErrorContext);
  
! 	EVALUATE_MESSAGE_DOMAIN(domain, context, true, true);
  
  	MemoryContextSwitchTo(oldcontext);
  	recursion_depth--;
  	return 0;					/* return value does not matter */
  }
  
  /*
   * errhidestmt --- optionally suppress STATEMENT: field of log entry
   *
elog_h.difftext/plain; name=elog_h.diffDownload
*** original/pgsql/src/include/utils/elog.h	2012-11-06 02:11:02.960963602 +0900
--- new/pgsql/src/include/utils/elog.h	2012-11-06 02:11:02.888953895 +0900
***************
*** 109,114 ****
--- 109,117 ----
  #define ereport(elevel, rest)	\
  	ereport_domain(elevel, TEXTDOMAIN, rest)
  
+ #define errcontext(...)	\
+ 	errcontext_domain(TEXTDOMAIN, __VA_ARGS__)
+ 
  #define TEXTDOMAIN NULL
  
  extern bool errstart(int elevel, const char *filename, int lineno,
***************
*** 173,182 ****
  __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2)));
  
  extern int
! errcontext(const char *fmt,...)
  /* This extension allows gcc to check the format string for consistency with
     the supplied arguments. */
! __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2)));
  
  extern int	errhidestmt(bool hide_stmt);
  
--- 176,185 ----
  __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2)));
  
  extern int
! errcontext_domain(const char *domain, const char *fmt,...)
  /* This extension allows gcc to check the format string for consistency with
     the supplied arguments. */
! __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3)));
  
  extern int	errhidestmt(bool hide_stmt);
  
#2Heikki Linnakangas
hlinnakangas@vmware.com
In reply to: Chen Huajun (#1)
Re: Fix errcontext() function

On 08.11.2012 07:59, Chen Huajun wrote:

I am sending patch for errcontext() function.

I use procedural languages to do some operation, but when error occurs
,the CONTEXT error messages from procedural languages doesn't display in
local language.

for example:
--------------------------------------------------------
postgres=# CREATE OR REPLACE FUNCTION logfunc3 (logtxt text) RETURNS
timestamp AS $$
postgres$# BEGIN
postgres$# select * from db;
postgres$# RETURN 'now';
postgres$# END;
postgres$# $$ LANGUAGE plpgsql;
CREATE FUNCTION
postgres=# select logfunc3('test');
ERROR: リレーション"db"は存在しません
行 1: select * from db
QUERY: select * from db
CONTEXT: PL/pgSQL function "logfunc3" line 3 at SQL ステートメント
--------------------------------------------------------
but,“CONTEXT: PL/pgSQL 関数 "logfunc3" の 3 行目の型 SQL ステートメント” is my expected.

There is the same problem in pl/perl and pl/python .

After checking and debuging the source code ,I found the reason.

The reason is that domian setted is wrong. For PL/pgSQL, domain "pgsql" should be setted, but domain setted is "postgres" .

So I considered to fix the bug by updating errcontext() funtion.

Unfortunately not all compilers support varargs macros. I bumped into
this in February, see
http://archives.postgresql.org/message-id/4F3B72E0.8040801@enterprisedb.com.
My last attempt to fix this was at
http://archives.postgresql.org/pgsql-hackers/2012-04/msg00812.php. That
patch is probably good to go, I just got busy with other things and
forgot about it back then. Can you take a look at that patch and see if
I missed anything, please?

- Heikki

#3Chen Huajun
chenhj@cn.fujitsu.com
In reply to: Heikki Linnakangas (#2)
Re: Fix errcontext() function

Heikki

Unfortunately not all compilers support varargs macros. I bumped into this in February, see http://archives.postgresql.org/message-id/4F3B72E0.8040801@enterprisedb.com. My last attempt to fix this was
at http://archives.postgresql.org/pgsql-hackers/2012-04/msg00812.php. That patch is probably good to go, I just got busy with other things and forgot about it back then. Can you take a look at that
patch and see if I missed anything, please?

- Heikki

I think you are right,although the number of changed place is a a little bit large.
Thanks for your answer!

Chen Huajun

#4Heikki Linnakangas
hlinnakangas@vmware.com
In reply to: Chen Huajun (#3)
Re: Fix errcontext() function

On 10.11.2012 11:46, Chen Huajun wrote:

Unfortunately not all compilers support varargs macros. I bumped into
this in February, see
http://archives.postgresql.org/message-id/4F3B72E0.8040801@enterprisedb.com.
My last attempt to fix this was
at http://archives.postgresql.org/pgsql-hackers/2012-04/msg00812.php.
That patch is probably good to go, I just got busy with other things
and forgot about it back then. Can you take a look at that
patch and see if I missed anything, please?

I think you are right,although the number of changed place is a a little
bit large.
Thanks for your answer!

Ok, I've committed this patch now, it will be fixed in 9.3. Thanks for
reminding me about this.

- Heikki

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Heikki Linnakangas (#4)
Re: Fix errcontext() function

Heikki Linnakangas wrote:

On 10.11.2012 11:46, Chen Huajun wrote:

Unfortunately not all compilers support varargs macros. I bumped into
this in February, see
http://archives.postgresql.org/message-id/4F3B72E0.8040801@enterprisedb.com.
My last attempt to fix this was
at http://archives.postgresql.org/pgsql-hackers/2012-04/msg00812.php.
That patch is probably good to go, I just got busy with other things
and forgot about it back then. Can you take a look at that
patch and see if I missed anything, please?

I think you are right,although the number of changed place is a a little
bit large.
Thanks for your answer!

Ok, I've committed this patch now, it will be fixed in 9.3. Thanks
for reminding me about this.

Hopefully you noticed that contrib is broken.

--

#6Heikki Linnakangas
hlinnakangas@vmware.com
In reply to: Alvaro Herrera (#5)
Re: Fix errcontext() function

On 12.11.2012 17:52, Alvaro Herrera wrote:

Hopefully you noticed that contrib is broken.

Oops.. fixed.

- Heikki