proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

Started by Pavel Stehulealmost 13 years ago13 messages

pavel.stehule@gmail.com

almost 13 years ago

1 attachment(s)

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN
outer_outer
─────────────
20
(1 row)

Ideas, comments?

Regards

Pavel Stehule

Attachments:

get_diagnostics_context_initial.patchapplication/octet-stream; name=get_diagnostics_context_initial.patchDownload

*** a/src/backend/utils/error/elog.c
--- b/src/backend/utils/error/elog.c
***************
*** 3124,3126 **** trace_recovery(int trace_level)
--- 3124,3180 ----
  
  	return trace_level;
  }
+ 
+ /*
+  * InvokeErrorCallbacks - invoke registrated error callback functions.
+  */
+ char *
+ InvokeErrorCallbacks(void)
+ {
+ 	ErrorData  *edata;
+ 	ErrorContextCallback *econtext;
+ 	MemoryContext oldcontext = CurrentMemoryContext;
+ 	char		*result;
+ 
+ 	/* this function should not be started in exception handler */
+ 	Assert(recursion_depth == 0);
+ 
+ 	if (++errordata_stack_depth >= ERRORDATA_STACK_SIZE)
+ 	{
+ 		errordata_stack_depth = -1;		/* make room on stack */
+ 		ereport(PANIC, (errmsg_internal("ERRORDATA_STACK_SIZE exceeded")));
+ 	}
+ 
+ 	/* Initialize data for this error frame */
+ 	edata = &errordata[errordata_stack_depth];
+ 	MemSet(edata, 0, sizeof(ErrorData));
+ 
+ 	/* Use ErrorContext as short living context */
+ 	MemoryContextSwitchTo(ErrorContext);
+ 
+ 	/*
+ 	 * Call any context callback functions.  Errors occurring in callback
+ 	 * functions will be treated as recursive errors --- this ensures we will
+ 	 * avoid infinite recursion (see errstart).
+ 	 */
+ 	for (econtext = error_context_stack;
+ 		 econtext != NULL;
+ 		 econtext = econtext->previous)
+ 		(*econtext->callback) (econtext->arg);
+ 
+ 	MemoryContextSwitchTo(oldcontext);
+ 	result = pstrdup(edata->context ? edata->context : "");
+ 
+ 	/*
+ 	 * Reset stack to empty.  The only case where it would be more than one
+ 	 * deep is if we serviced an error that interrupted construction of
+ 	 * another message.  We assume control escaped out of that message
+ 	 * construction and won't ever go back.
+ 	 */
+ 	errordata_stack_depth = -1;
+ 	recursion_depth = 0;
+ 	/* Delete all data in ErrorContext */
+ 	MemoryContextResetAndDeleteChildren(ErrorContext);
+ 
+ 	return result;
+ }
*** a/src/include/utils/elog.h
--- b/src/include/utils/elog.h
***************
*** 406,411 **** extern void FlushErrorState(void);
--- 406,413 ----
  extern void ReThrowError(ErrorData *edata) __attribute__((noreturn));
  extern void pg_re_throw(void) __attribute__((noreturn));
  
+ extern char *InvokeErrorCallbacks(void);
+ 
  /* Hook for intercepting messages before they are sent to the server log */
  typedef void (*emit_log_hook_type) (ErrorData *edata);
  extern PGDLLIMPORT emit_log_hook_type emit_log_hook;
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***************
*** 1599,1604 **** exec_stmt_getdiag(PLpgSQL_execstate *estate, PLpgSQL_stmt_getdiag *stmt)
--- 1599,1616 ----
  									 estate->cur_error->schema_name);
  				break;
  
+ 			case PLPGSQL_GETDIAG_CONTEXT:
+ 				{
+ 					char *context;
+ 
+ 					context = InvokeErrorCallbacks();
+ 
+ 					exec_assign_c_string(estate, var,
+ 										context);
+ 					pfree(context);
+ 				}
+ 				break;
+ 
  			default:
  				elog(ERROR, "unrecognized diagnostic item kind: %d",
  					 diag_item->kind);
*** a/src/pl/plpgsql/src/pl_funcs.c
--- b/src/pl/plpgsql/src/pl_funcs.c
***************
*** 273,278 **** plpgsql_getdiag_kindname(int kind)
--- 273,280 ----
  {
  	switch (kind)
  	{
+ 		case PLPGSQL_GETDIAG_CONTEXT:
+ 			return "PG_CONTEXT";
  		case PLPGSQL_GETDIAG_ROW_COUNT:
  			return "ROW_COUNT";
  		case PLPGSQL_GETDIAG_RESULT_OID:
*** a/src/pl/plpgsql/src/pl_gram.y
--- b/src/pl/plpgsql/src/pl_gram.y
***************
*** 301,306 **** static	List			*read_raise_options(void);
--- 301,307 ----
  %token <keyword>	K_OPTION
  %token <keyword>	K_OR
  %token <keyword>	K_PERFORM
+ %token <keyword>	K_PG_CONTEXT
  %token <keyword>	K_PG_EXCEPTION_CONTEXT
  %token <keyword>	K_PG_EXCEPTION_DETAIL
  %token <keyword>	K_PG_EXCEPTION_HINT
***************
*** 889,894 **** stmt_getdiag	: K_GET getdiag_area_opt K_DIAGNOSTICS getdiag_list ';'
--- 890,896 ----
  								/* these fields are disallowed in stacked case */
  								case PLPGSQL_GETDIAG_ROW_COUNT:
  								case PLPGSQL_GETDIAG_RESULT_OID:
+ 								case PLPGSQL_GETDIAG_CONTEXT:
  									if (new->is_stacked)
  										ereport(ERROR,
  												(errcode(ERRCODE_SYNTAX_ERROR),
***************
*** 966,971 **** getdiag_item :
--- 968,976 ----
  						int	tok = yylex();
  
  						if (tok_is_keyword(tok, &yylval,
+ 										   K_PG_CONTEXT, "pg_context"))
+ 							$$ = PLPGSQL_GETDIAG_CONTEXT;
+ 						else if (tok_is_keyword(tok, &yylval,
  										   K_ROW_COUNT, "row_count"))
  							$$ = PLPGSQL_GETDIAG_ROW_COUNT;
  						else if (tok_is_keyword(tok, &yylval,
***************
*** 2280,2285 **** unreserved_keyword	:
--- 2285,2291 ----
  				| K_NO
  				| K_NOTICE
  				| K_OPTION
+ 				| K_PG_CONTEXT
  				| K_PG_EXCEPTION_CONTEXT
  				| K_PG_EXCEPTION_DETAIL
  				| K_PG_EXCEPTION_HINT
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
***************
*** 133,138 **** static const ScanKeyword unreserved_keywords[] = {
--- 133,139 ----
  	PG_KEYWORD("no", K_NO, UNRESERVED_KEYWORD)
  	PG_KEYWORD("notice", K_NOTICE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("option", K_OPTION, UNRESERVED_KEYWORD)
+ 	PG_KEYWORD("pg_context", K_PG_CONTEXT, UNRESERVED_KEYWORD)
  	PG_KEYWORD("pg_exception_context", K_PG_EXCEPTION_CONTEXT, UNRESERVED_KEYWORD)
  	PG_KEYWORD("pg_exception_detail", K_PG_EXCEPTION_DETAIL, UNRESERVED_KEYWORD)
  	PG_KEYWORD("pg_exception_hint", K_PG_EXCEPTION_HINT, UNRESERVED_KEYWORD)
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 122,127 **** enum
--- 122,128 ----
   */
  enum
  {
+ 	PLPGSQL_GETDIAG_CONTEXT,
  	PLPGSQL_GETDIAG_ROW_COUNT,
  	PLPGSQL_GETDIAG_RESULT_OID,
  	PLPGSQL_GETDIAG_ERROR_CONTEXT,
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
***************
*** 4897,4899 **** ERROR:  value for domain orderedarray violates check constraint "sorted"
--- 4897,4947 ----
  CONTEXT:  PL/pgSQL function testoa(integer,integer,integer) line 5 at assignment
  drop function arrayassign1();
  drop function testoa(x1 int, x2 int, x3 int);
+ -- access to call stack
+ create function inner_func(int)
+ returns int as $$
+ declare _context text;
+ begin
+   get diagnostics _context = pg_context;
+   raise notice '***%***', _context;
+   return 2 * $1;
+ end;
+ $$ language plpgsql;
+ create or replace function outer_func(int)
+ returns int as $$
+ begin
+   return inner_func($1);
+ end;
+ $$ language plpgsql;
+ create or replace function outer_outer_func(int)
+ returns int as $$
+ begin
+   return outer_func($1);
+ end;
+ $$ language plpgsql;
+ select outer_outer_func(10);
+ NOTICE:  ***PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
+ PL/pgSQL function outer_func(integer) line 3 at RETURN
+ PL/pgSQL function outer_outer_func(integer) line 3 at RETURN***
+ CONTEXT:  PL/pgSQL function outer_func(integer) line 3 at RETURN
+ PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
+  outer_outer_func 
+ ------------------
+                20
+ (1 row)
+ 
+ -- repeated call should to work
+ select outer_outer_func(20);
+ NOTICE:  ***PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
+ PL/pgSQL function outer_func(integer) line 3 at RETURN
+ PL/pgSQL function outer_outer_func(integer) line 3 at RETURN***
+ CONTEXT:  PL/pgSQL function outer_func(integer) line 3 at RETURN
+ PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
+  outer_outer_func 
+ ------------------
+                40
+ (1 row)
+ 
+ drop function outer_outer_func(int);
+ drop function outer_func(int);
+ drop function inner_func(int);
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
***************
*** 3880,3882 **** select testoa(1,2,1); -- fail at update
--- 3880,3915 ----
  
  drop function arrayassign1();
  drop function testoa(x1 int, x2 int, x3 int);
+ 
+ -- access to call stack
+ create function inner_func(int)
+ returns int as $$
+ declare _context text;
+ begin
+   get diagnostics _context = pg_context;
+   raise notice '***%***', _context;
+   return 2 * $1;
+ end;
+ $$ language plpgsql;
+ 
+ create or replace function outer_func(int)
+ returns int as $$
+ begin
+   return inner_func($1);
+ end;
+ $$ language plpgsql;
+ 
+ create or replace function outer_outer_func(int)
+ returns int as $$
+ begin
+   return outer_func($1);
+ end;
+ $$ language plpgsql;
+ 
+ select outer_outer_func(10);
+ -- repeated call should to work
+ select outer_outer_func(20);
+ 
+ drop function outer_outer_func(int);
+ drop function outer_func(int);
+ drop function inner_func(int);

Jim Nasby

jim@nasby.net

almost 13 years ago

In reply to: Pavel Stehule (#1)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

On 2/2/13 3:23 AM, Pavel Stehule wrote:

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN

That could *really* stand for some indentation in the output instead of the *** business. But other than that, this definitely seems useful. I had no idea _context was even an option... :/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Rushabh Lathia

rushabh.lathia@gmail.com

over 12 years ago

In reply to: Pavel Stehule (#1)

1 attachment(s)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

Hi,

Use of this feature is to get call stack for debugging without raising
exception. This definitely seems very useful.

Here are my comments about the submitted patch:

Patch gets applied cleanly (there are couple of white space warning but
that's
into test case file). make and make install too went smooth. make check
was smooth too. Did some manual testing about feature and its went smooth.

However would like to share couple of points:

1) I noticed that you did the initialization of edata manually, just
wondering
can't we directly use CopyErrorData() which will do that job ?

I found that inside CopyErrorData() there is Assert:

Assert(CurrentMemoryContext != ErrorContext);

And in the patch we are setting using ErrorContext as short living context,
is
it the reason of not using CopyErrorData() ?

2) To reset stack to empty, FlushErrorState() can be used.

3) I was think about the usability of the feature and wondering how about if
we add some header to the stack, so user can differentiate between STACK and
normal NOTICE message ?

Something like:

postgres=# select outer_outer_func(10);
NOTICE: ----- Call Stack -----
PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
outer_outer_func
------------------
20
(1 row)

I worked on point 2) and 3) and PFA patch for reference.

Thanks,
Rushabh

On Sat, Feb 2, 2013 at 2:53 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN
outer_outer
─────────────
20
(1 row)

Ideas, comments?

Regards

Pavel Stehule

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Rushabh Lathia

Attachments:

get_diagnostics_context_initial_v2.patchapplication/octet-stream; name=get_diagnostics_context_initial_v2.patchDownload

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 7f03f41..feb9984 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -3120,3 +3120,53 @@ trace_recovery(int trace_level)
 
 	return trace_level;
 }
+
+/*
+ * InvokeErrorCallbacks - invoke registrated error callback functions.
+ */
+char *
+InvokeErrorCallbacks(void)
+{
+	ErrorData  *edata;
+	ErrorContextCallback *econtext;
+	MemoryContext oldcontext = CurrentMemoryContext;
+	StringInfoData sinfo;
+
+	/* this function should not be started in exception handler */
+	Assert(recursion_depth == 0);
+
+	if (++errordata_stack_depth >= ERRORDATA_STACK_SIZE)
+	{
+		errordata_stack_depth = -1;		/* make room on stack */
+		ereport(PANIC, (errmsg_internal("ERRORDATA_STACK_SIZE exceeded")));
+	}
+
+	/* Initialize data for this error frame */
+	edata = &errordata[errordata_stack_depth];
+	MemSet(edata, 0, sizeof(ErrorData));
+
+	/* Use ErrorContext as short living context */
+	MemoryContextSwitchTo(ErrorContext);
+
+	/*
+	 * Call any context callback functions.  Errors occurring in callback
+	 * functions will be treated as recursive errors --- this ensures we will
+	 * avoid infinite recursion (see errstart).
+	 */
+	for (econtext = error_context_stack;
+		 econtext != NULL;
+		 econtext = econtext->previous)
+		(*econtext->callback) (econtext->arg);
+
+	MemoryContextSwitchTo(oldcontext);
+
+	initStringInfo(&sinfo);
+
+	appendStringInfoString(&sinfo, "----- Call Stack -----\n");
+	if (edata->context)
+		appendStringInfoString(&sinfo, edata->context);
+
+	FlushErrorState();
+
+	return sinfo.data;
+}
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 85bd2fd..fca826d 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -406,6 +406,8 @@ extern void FlushErrorState(void);
 extern void ReThrowError(ErrorData *edata) __attribute__((noreturn));
 extern void pg_re_throw(void) __attribute__((noreturn));
 
+extern char *InvokeErrorCallbacks(void);
+
 /* Hook for intercepting messages before they are sent to the server log */
 typedef void (*emit_log_hook_type) (ErrorData *edata);
 extern PGDLLIMPORT emit_log_hook_type emit_log_hook;
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 70e67d9..139c5e7 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -1574,6 +1574,18 @@ exec_stmt_getdiag(PLpgSQL_execstate *estate, PLpgSQL_stmt_getdiag *stmt)
 									 estate->cur_error->message);
 				break;
 
+			case PLPGSQL_GETDIAG_CONTEXT:
+				{
+					char *context;
+
+					context = InvokeErrorCallbacks();
+
+					exec_assign_c_string(estate, var,
+										context);
+					pfree(context);
+				}
+				break;
+
 			default:
 				elog(ERROR, "unrecognized diagnostic item kind: %d",
 					 diag_item->kind);
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index 9d561c2..36f144a 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -273,6 +273,8 @@ plpgsql_getdiag_kindname(int kind)
 {
 	switch (kind)
 	{
+		case PLPGSQL_GETDIAG_CONTEXT:
+			return "PG_CONTEXT";
 		case PLPGSQL_GETDIAG_ROW_COUNT:
 			return "ROW_COUNT";
 		case PLPGSQL_GETDIAG_RESULT_OID:
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index a790ee3..0ee9e2f 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -298,6 +298,7 @@ static	List			*read_raise_options(void);
 %token <keyword>	K_OPTION
 %token <keyword>	K_OR
 %token <keyword>	K_PERFORM
+%token <keyword>	K_PG_CONTEXT
 %token <keyword>	K_PG_EXCEPTION_CONTEXT
 %token <keyword>	K_PG_EXCEPTION_DETAIL
 %token <keyword>	K_PG_EXCEPTION_HINT
@@ -884,6 +885,7 @@ stmt_getdiag	: K_GET getdiag_area_opt K_DIAGNOSTICS getdiag_list ';'
 								/* these fields are disallowed in stacked case */
 								case PLPGSQL_GETDIAG_ROW_COUNT:
 								case PLPGSQL_GETDIAG_RESULT_OID:
+								case PLPGSQL_GETDIAG_CONTEXT:
 									if (new->is_stacked)
 										ereport(ERROR,
 												(errcode(ERRCODE_SYNTAX_ERROR),
@@ -956,6 +958,9 @@ getdiag_item :
 						int	tok = yylex();
 
 						if (tok_is_keyword(tok, &yylval,
+										   K_PG_CONTEXT, "pg_context"))
+							$$ = PLPGSQL_GETDIAG_CONTEXT;
+						else if (tok_is_keyword(tok, &yylval,
 										   K_ROW_COUNT, "row_count"))
 							$$ = PLPGSQL_GETDIAG_ROW_COUNT;
 						else if (tok_is_keyword(tok, &yylval,
@@ -2252,6 +2257,7 @@ unreserved_keyword	:
 				| K_NO
 				| K_NOTICE
 				| K_OPTION
+				| K_PG_CONTEXT
 				| K_PG_EXCEPTION_CONTEXT
 				| K_PG_EXCEPTION_DETAIL
 				| K_PG_EXCEPTION_HINT
diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
index 9b6f57e..19aca1d 100644
--- a/src/pl/plpgsql/src/pl_scanner.c
+++ b/src/pl/plpgsql/src/pl_scanner.c
@@ -130,6 +130,7 @@ static const ScanKeyword unreserved_keywords[] = {
 	PG_KEYWORD("no", K_NO, UNRESERVED_KEYWORD)
 	PG_KEYWORD("notice", K_NOTICE, UNRESERVED_KEYWORD)
 	PG_KEYWORD("option", K_OPTION, UNRESERVED_KEYWORD)
+	PG_KEYWORD("pg_context", K_PG_CONTEXT, UNRESERVED_KEYWORD)
 	PG_KEYWORD("pg_exception_context", K_PG_EXCEPTION_CONTEXT, UNRESERVED_KEYWORD)
 	PG_KEYWORD("pg_exception_detail", K_PG_EXCEPTION_DETAIL, UNRESERVED_KEYWORD)
 	PG_KEYWORD("pg_exception_hint", K_PG_EXCEPTION_HINT, UNRESERVED_KEYWORD)
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 5cc44a0..6d45237 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -122,6 +122,7 @@ enum
  */
 enum
 {
+	PLPGSQL_GETDIAG_CONTEXT,
 	PLPGSQL_GETDIAG_ROW_COUNT,
 	PLPGSQL_GETDIAG_RESULT_OID,
 	PLPGSQL_GETDIAG_ERROR_CONTEXT,
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index fdd8c6b..98e8cb8 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -4863,3 +4863,51 @@ ERROR:  value for domain orderedarray violates check constraint "sorted"
 CONTEXT:  PL/pgSQL function testoa(integer,integer,integer) line 5 at assignment
 drop function arrayassign1();
 drop function testoa(x1 int, x2 int, x3 int);
+-- access to call stack
+create function inner_func(int)
+returns int as $$
+declare _context text;
+begin
+  get diagnostics _context = pg_context;
+  raise notice '***%***', _context;
+  return 2 * $1;
+end;
+$$ language plpgsql;
+create or replace function outer_func(int)
+returns int as $$
+begin
+  return inner_func($1);
+end;
+$$ language plpgsql;
+create or replace function outer_outer_func(int)
+returns int as $$
+begin
+  return outer_func($1);
+end;
+$$ language plpgsql;
+select outer_outer_func(10);
+NOTICE:  ***PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
+PL/pgSQL function outer_func(integer) line 3 at RETURN
+PL/pgSQL function outer_outer_func(integer) line 3 at RETURN***
+CONTEXT:  PL/pgSQL function outer_func(integer) line 3 at RETURN
+PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
+ outer_outer_func 
+------------------
+               20
+(1 row)
+
+-- repeated call should to work
+select outer_outer_func(20);
+NOTICE:  ***PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
+PL/pgSQL function outer_func(integer) line 3 at RETURN
+PL/pgSQL function outer_outer_func(integer) line 3 at RETURN***
+CONTEXT:  PL/pgSQL function outer_func(integer) line 3 at RETURN
+PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
+ outer_outer_func 
+------------------
+               40
+(1 row)
+
+drop function outer_outer_func(int);
+drop function outer_func(int);
+drop function inner_func(int);
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 017bd0b..643c0fc 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -3848,3 +3848,36 @@ select testoa(1,2,1); -- fail at update
 
 drop function arrayassign1();
 drop function testoa(x1 int, x2 int, x3 int);
+
+-- access to call stack
+create function inner_func(int)
+returns int as $$
+declare _context text;
+begin
+  get diagnostics _context = pg_context;
+  raise notice '***%***', _context;
+  return 2 * $1;
+end;
+$$ language plpgsql;
+
+create or replace function outer_func(int)
+returns int as $$
+begin
+  return inner_func($1);
+end;
+$$ language plpgsql;
+
+create or replace function outer_outer_func(int)
+returns int as $$
+begin
+  return outer_func($1);
+end;
+$$ language plpgsql;
+
+select outer_outer_func(10);
+-- repeated call should to work
+select outer_outer_func(20);
+
+drop function outer_outer_func(int);
+drop function outer_func(int);
+drop function inner_func(int);

Pavel Stehule

pavel.stehule@gmail.com

over 12 years ago

In reply to: Rushabh Lathia (#3)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

Hello

This is fragment ofmy old code from orafce package - it is functional,
but it is written little bit more generic due impossible access to
static variables (in elog.c)

static char*
dbms_utility_format_call_stack(char mode)
{
MemoryContext oldcontext = CurrentMemoryContext;
ErrorData *edata;
ErrorContextCallback *econtext;
StringInfo sinfo;

#if PG_VERSION_NUM >= 80400
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO, TEXTDOMAIN);
#else
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO);
#endif

MemoryContextSwitchTo(oldcontext);

for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext->previous)
(*econtext->callback) (econtext->arg);

edata = CopyErrorData();
FlushErrorState();

https://github.com/orafce/orafce/blob/master/utility.c

2013/6/24 Rushabh Lathia <rushabh.lathia@gmail.com>:

Hi,

Use of this feature is to get call stack for debugging without raising
exception. This definitely seems very useful.

Here are my comments about the submitted patch:

Patch gets applied cleanly (there are couple of white space warning but
that's
into test case file). make and make install too went smooth. make check
was smooth too. Did some manual testing about feature and its went smooth.

However would like to share couple of points:

My main motive is concentration to stack info string only. So I didn't
use err_start function, and I didn't use CopyErrorData too. These
routines does lot of other work.

1) I noticed that you did the initialization of edata manually, just
wondering
can't we directly use CopyErrorData() which will do that job ?

yes, we can, but in this context on "context" field is interesting for us.

I found that inside CopyErrorData() there is Assert:

Assert(CurrentMemoryContext != ErrorContext);

And in the patch we are setting using ErrorContext as short living context,
is
it the reason of not using CopyErrorData() ?

it was not a primary reason, but sure - this routine is not designed
for direct using from elog module. Next, we can save one palloc call.

2) To reset stack to empty, FlushErrorState() can be used.

yes, it can be. I use explicit rows due different semantics, although
it does same things. FlushErrorState some global ErrorState and it is
used from other modules. I did a reset of ErrorContext. I fill a small
difference there - so FlushErrorState is not best name for my purpose.
What about modification:

static void
resetErrorStack()
{
errordata_stack_depth = -1;
recursion_depth = 0;
/* Delete all data in ErrorContext */
MemoryContextResetAndDeleteChildren(ErrorContext);
}

and then call in InvokeErrorCallbacks -- resetErrorStack()

and rewrote flushErrorState like

void FlushErrorState(void)
{
/* reset ErrorStack is enough */
resetErrorStack();
}

???

but I can live well with direct call of FlushErrorState() - it is only
minor change.

3) I was think about the usability of the feature and wondering how about if
we add some header to the stack, so user can differentiate between STACK and
normal NOTICE message ?

Something like:

postgres=# select outer_outer_func(10);
NOTICE: ----- Call Stack -----
PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
outer_outer_func
------------------
20
(1 row)

I understand, but I don't think so it is good idea. Sometimes, you
would to use context for different purposes than debug log - for
example - you should to identify top most call or near call - and any
additional lines means some little bit more difficult parsing. You can
add this line simply (if you want)

RAISE NOTICE e'--- Call Stack ---\n%', stack

but there are more use cases, where you would not this information (or
you can use own header (different language)), and better returns only
clean data.

I worked on point 2) and 3) and PFA patch for reference.

*1 CopyErrorData does one useless palloc more and it is too generic,

*2 it is possible - I have not strong opinion

*3 no, I would to return data in most simply and clean form without any sugar

What do you think?

Regards

Pavel

Thanks,
Rushabh

On Sat, Feb 2, 2013 at 2:53 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN
outer_outer
─────────────
20
(1 row)

Ideas, comments?

Regards

Pavel Stehule

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Rushabh Lathia

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Rushabh Lathia

rushabh.lathia@gmail.com

over 12 years ago

In reply to: Pavel Stehule (#4)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule <pavel.stehule@gmail.com>wrote:

Hello

This is fragment ofmy old code from orafce package - it is functional,
but it is written little bit more generic due impossible access to
static variables (in elog.c)

static char*
dbms_utility_format_call_stack(char mode)
{
MemoryContext oldcontext = CurrentMemoryContext;
ErrorData *edata;
ErrorContextCallback *econtext;
StringInfo sinfo;

#if PG_VERSION_NUM >= 80400
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO, TEXTDOMAIN);
#else
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO);
#endif

MemoryContextSwitchTo(oldcontext);

for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext->previous)
(*econtext->callback) (econtext->arg);

edata = CopyErrorData();
FlushErrorState();

https://github.com/orafce/orafce/blob/master/utility.c

2013/6/24 Rushabh Lathia <rushabh.lathia@gmail.com>:

Hi,

Use of this feature is to get call stack for debugging without raising
exception. This definitely seems very useful.

Here are my comments about the submitted patch:

Patch gets applied cleanly (there are couple of white space warning but
that's
into test case file). make and make install too went smooth. make check
was smooth too. Did some manual testing about feature and its went

smooth.

However would like to share couple of points:

My main motive is concentration to stack info string only. So I didn't
use err_start function, and I didn't use CopyErrorData too. These
routines does lot of other work.

1) I noticed that you did the initialization of edata manually, just
wondering
can't we directly use CopyErrorData() which will do that job ?

yes, we can, but in this context on "context" field is interesting for us.

I found that inside CopyErrorData() there is Assert:

Assert(CurrentMemoryContext != ErrorContext);

And in the patch we are setting using ErrorContext as short living

context,

is
it the reason of not using CopyErrorData() ?

it was not a primary reason, but sure - this routine is not designed
for direct using from elog module. Next, we can save one palloc call.

Hmm ok.

2) To reset stack to empty, FlushErrorState() can be used.

yes, it can be. I use explicit rows due different semantics, although
it does same things. FlushErrorState some global ErrorState and it is
used from other modules. I did a reset of ErrorContext. I fill a small
difference there - so FlushErrorState is not best name for my purpose.
What about modification:

static void
resetErrorStack()
{
errordata_stack_depth = -1;
recursion_depth = 0;
/* Delete all data in ErrorContext */
MemoryContextResetAndDeleteChildren(ErrorContext);
}

and then call in InvokeErrorCallbacks -- resetErrorStack()

and rewrote flushErrorState like

void FlushErrorState(void)
{
/* reset ErrorStack is enough */
resetErrorStack();
}

???

Nope. rather then that I would still prefer direct call of
FlushErrorState().

but I can live well with direct call of FlushErrorState() - it is only
minor change.

3) I was think about the usability of the feature and wondering how

about if

we add some header to the stack, so user can differentiate between STACK

and

normal NOTICE message ?

Something like:

postgres=# select outer_outer_func(10);
NOTICE: ----- Call Stack -----
PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
outer_outer_func
------------------
20
(1 row)

I understand, but I don't think so it is good idea. Sometimes, you
would to use context for different purposes than debug log - for
example - you should to identify top most call or near call - and any
additional lines means some little bit more difficult parsing. You can
add this line simply (if you want)

RAISE NOTICE e'--- Call Stack ---\n%', stack

but there are more use cases, where you would not this information (or
you can use own header (different language)), and better returns only
clean data.

I don't know but I still feel like given header from function it self would
be
nice to have things. Because it will help user to differentiate between
STACK and normal NOTICE context message.

I worked on point 2) and 3) and PFA patch for reference.

so

*1 CopyErrorData does one useless palloc more and it is too generic,

*2 it is possible - I have not strong opinion

Prefer FlushErrorState() call.

*3 no, I would to return data in most simply and clean form without any
sugar

As a user perspective it would be nice to have sugar layer around.

What do you think?

Others or committer having any opinion ?

Regards

Pavel

Thanks,
Rushabh

On Sat, Feb 2, 2013 at 2:53 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN
outer_outer
─────────────
20
(1 row)

Ideas, comments?

Regards

Pavel Stehule

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Rushabh Lathia

--
Rushabh Lathia

Pavel Stehule

pavel.stehule@gmail.com

over 12 years ago

In reply to: Rushabh Lathia (#5)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

2013/6/25 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hello

This is fragment ofmy old code from orafce package - it is functional,
but it is written little bit more generic due impossible access to
static variables (in elog.c)

static char*
dbms_utility_format_call_stack(char mode)
{
MemoryContext oldcontext = CurrentMemoryContext;
ErrorData *edata;
ErrorContextCallback *econtext;
StringInfo sinfo;

#if PG_VERSION_NUM >= 80400
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO, TEXTDOMAIN);
#else
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO);
#endif

MemoryContextSwitchTo(oldcontext);

for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext->previous)
(*econtext->callback) (econtext->arg);

edata = CopyErrorData();
FlushErrorState();

https://github.com/orafce/orafce/blob/master/utility.c

2013/6/24 Rushabh Lathia <rushabh.lathia@gmail.com>:

Hi,

Use of this feature is to get call stack for debugging without raising
exception. This definitely seems very useful.

Here are my comments about the submitted patch:

Patch gets applied cleanly (there are couple of white space warning but
that's
into test case file). make and make install too went smooth. make check
was smooth too. Did some manual testing about feature and its went
smooth.

However would like to share couple of points:

My main motive is concentration to stack info string only. So I didn't
use err_start function, and I didn't use CopyErrorData too. These
routines does lot of other work.

1) I noticed that you did the initialization of edata manually, just
wondering
can't we directly use CopyErrorData() which will do that job ?

yes, we can, but in this context on "context" field is interesting for us.

I found that inside CopyErrorData() there is Assert:

Assert(CurrentMemoryContext != ErrorContext);

And in the patch we are setting using ErrorContext as short living
context,
is
it the reason of not using CopyErrorData() ?

it was not a primary reason, but sure - this routine is not designed
for direct using from elog module. Next, we can save one palloc call.

Hmm ok.

2) To reset stack to empty, FlushErrorState() can be used.

yes, it can be. I use explicit rows due different semantics, although
it does same things. FlushErrorState some global ErrorState and it is
used from other modules. I did a reset of ErrorContext. I fill a small
difference there - so FlushErrorState is not best name for my purpose.
What about modification:

static void
resetErrorStack()
{
errordata_stack_depth = -1;
recursion_depth = 0;
/* Delete all data in ErrorContext */
MemoryContextResetAndDeleteChildren(ErrorContext);
}

and then call in InvokeErrorCallbacks -- resetErrorStack()

and rewrote flushErrorState like

void FlushErrorState(void)
{
/* reset ErrorStack is enough */
resetErrorStack();
}

???

Nope. rather then that I would still prefer direct call of
FlushErrorState().

but I can live well with direct call of FlushErrorState() - it is only
minor change.

3) I was think about the usability of the feature and wondering how
about if
we add some header to the stack, so user can differentiate between STACK
and
normal NOTICE message ?

Something like:

postgres=# select outer_outer_func(10);
NOTICE: ----- Call Stack -----
PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
outer_outer_func
------------------
20
(1 row)

I understand, but I don't think so it is good idea. Sometimes, you
would to use context for different purposes than debug log - for
example - you should to identify top most call or near call - and any
additional lines means some little bit more difficult parsing. You can
add this line simply (if you want)

RAISE NOTICE e'--- Call Stack ---\n%', stack

but there are more use cases, where you would not this information (or
you can use own header (different language)), and better returns only
clean data.

I don't know but I still feel like given header from function it self would
be
nice to have things. Because it will help user to differentiate between
STACK and normal NOTICE context message.

I worked on point 2) and 3) and PFA patch for reference.

so

*1 CopyErrorData does one useless palloc more and it is too generic,

Ok

*2 it is possible - I have not strong opinion

Prefer FlushErrorState() call.

*3 no, I would to return data in most simply and clean form without any
sugar

As a user perspective it would be nice to have sugar layer around.

I understand, but disagree - strings are simply joined and is
difficult to separate it. I have strong opinion here.

* a reason for this construct is not only using in debug notices
* sugar is not used in GET DIAGNOSTICS statement ever - so possible
introduction is not consistent with other

Regards

Pavel

What do you think?

Others or committer having any opinion ?

???

Regards

Pavel

Thanks,
Rushabh

On Sat, Feb 2, 2013 at 2:53 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET
DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN
outer_outer
─────────────
20
(1 row)

Ideas, comments?

Regards

Pavel Stehule

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Rushabh Lathia

--
Rushabh Lathia

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Rushabh Lathia

rushabh.lathia@gmail.com

over 12 years ago

In reply to: Pavel Stehule (#6)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

On Tue, Jun 25, 2013 at 11:09 AM, Pavel Stehule <pavel.stehule@gmail.com>wrote:

2013/6/25 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hello

This is fragment ofmy old code from orafce package - it is functional,
but it is written little bit more generic due impossible access to
static variables (in elog.c)

static char*
dbms_utility_format_call_stack(char mode)
{
MemoryContext oldcontext = CurrentMemoryContext;
ErrorData *edata;
ErrorContextCallback *econtext;
StringInfo sinfo;

#if PG_VERSION_NUM >= 80400
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO,

TEXTDOMAIN);

#else
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO);
#endif

MemoryContextSwitchTo(oldcontext);

for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext->previous)
(*econtext->callback) (econtext->arg);

edata = CopyErrorData();
FlushErrorState();

https://github.com/orafce/orafce/blob/master/utility.c

2013/6/24 Rushabh Lathia <rushabh.lathia@gmail.com>:

Hi,

Use of this feature is to get call stack for debugging without

raising

exception. This definitely seems very useful.

Here are my comments about the submitted patch:

Patch gets applied cleanly (there are couple of white space warning

but

that's
into test case file). make and make install too went smooth. make

check

was smooth too. Did some manual testing about feature and its went
smooth.

However would like to share couple of points:

My main motive is concentration to stack info string only. So I didn't
use err_start function, and I didn't use CopyErrorData too. These
routines does lot of other work.

1) I noticed that you did the initialization of edata manually, just
wondering
can't we directly use CopyErrorData() which will do that job ?

yes, we can, but in this context on "context" field is interesting for

us.

I found that inside CopyErrorData() there is Assert:

Assert(CurrentMemoryContext != ErrorContext);

And in the patch we are setting using ErrorContext as short living
context,
is
it the reason of not using CopyErrorData() ?

it was not a primary reason, but sure - this routine is not designed
for direct using from elog module. Next, we can save one palloc call.

Hmm ok.

2) To reset stack to empty, FlushErrorState() can be used.

yes, it can be. I use explicit rows due different semantics, although
it does same things. FlushErrorState some global ErrorState and it is
used from other modules. I did a reset of ErrorContext. I fill a small
difference there - so FlushErrorState is not best name for my purpose.
What about modification:

static void
resetErrorStack()
{
errordata_stack_depth = -1;
recursion_depth = 0;
/* Delete all data in ErrorContext */
MemoryContextResetAndDeleteChildren(ErrorContext);
}

and then call in InvokeErrorCallbacks -- resetErrorStack()

and rewrote flushErrorState like

void FlushErrorState(void)
{
/* reset ErrorStack is enough */
resetErrorStack();
}

???

Nope. rather then that I would still prefer direct call of
FlushErrorState().

but I can live well with direct call of FlushErrorState() - it is only
minor change.

3) I was think about the usability of the feature and wondering how
about if
we add some header to the stack, so user can differentiate between

STACK

and
normal NOTICE message ?

Something like:

postgres=# select outer_outer_func(10);
NOTICE: ----- Call Stack -----
PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
outer_outer_func
------------------
20
(1 row)

I understand, but I don't think so it is good idea. Sometimes, you
would to use context for different purposes than debug log - for
example - you should to identify top most call or near call - and any
additional lines means some little bit more difficult parsing. You can
add this line simply (if you want)

RAISE NOTICE e'--- Call Stack ---\n%', stack

but there are more use cases, where you would not this information (or
you can use own header (different language)), and better returns only
clean data.

I don't know but I still feel like given header from function it self

would

be
nice to have things. Because it will help user to differentiate between
STACK and normal NOTICE context message.

I worked on point 2) and 3) and PFA patch for reference.

so

*1 CopyErrorData does one useless palloc more and it is too generic,

Ok

*2 it is possible - I have not strong opinion

Prefer FlushErrorState() call.

ok

*3 no, I would to return data in most simply and clean form without any
sugar

As a user perspective it would be nice to have sugar layer around.

I understand, but disagree - strings are simply joined and is
difficult to separate it. I have strong opinion here.

* a reason for this construct is not only using in debug notices
* sugar is not used in GET DIAGNOSTICS statement ever - so possible
introduction is not consistent with other

Hmm because sugar is not used in GET DIAGNOSTICS statement ever,
I think its should be fine.

How about point 2) To reset stack to empty, FlushErrorState() can be used. ?

Regards

Pavel

What do you think?

Others or committer having any opinion ?

???

Regards

Pavel

Thanks,
Rushabh

On Sat, Feb 2, 2013 at 2:53 PM, Pavel Stehule <

pavel.stehule@gmail.com>

wrote:

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET
DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN
outer_outer
─────────────
20
(1 row)

Ideas, comments?

Regards

Pavel Stehule

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Rushabh Lathia

--
Rushabh Lathia

--
Rushabh Lathia

Pavel Stehule

pavel.stehule@gmail.com

over 12 years ago

In reply to: Rushabh Lathia (#7)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

2013/6/26 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Tue, Jun 25, 2013 at 11:09 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2013/6/25 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hello

This is fragment ofmy old code from orafce package - it is functional,
but it is written little bit more generic due impossible access to
static variables (in elog.c)

static char*
dbms_utility_format_call_stack(char mode)
{
MemoryContext oldcontext = CurrentMemoryContext;
ErrorData *edata;
ErrorContextCallback *econtext;
StringInfo sinfo;

#if PG_VERSION_NUM >= 80400
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO,
TEXTDOMAIN);
#else
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO);
#endif

MemoryContextSwitchTo(oldcontext);

for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext->previous)
(*econtext->callback) (econtext->arg);

edata = CopyErrorData();
FlushErrorState();

https://github.com/orafce/orafce/blob/master/utility.c

2013/6/24 Rushabh Lathia <rushabh.lathia@gmail.com>:

Hi,

Use of this feature is to get call stack for debugging without
raising
exception. This definitely seems very useful.

Here are my comments about the submitted patch:

Patch gets applied cleanly (there are couple of white space warning
but
that's
into test case file). make and make install too went smooth. make
check
was smooth too. Did some manual testing about feature and its went
smooth.

However would like to share couple of points:

My main motive is concentration to stack info string only. So I didn't
use err_start function, and I didn't use CopyErrorData too. These
routines does lot of other work.

1) I noticed that you did the initialization of edata manually, just
wondering
can't we directly use CopyErrorData() which will do that job ?

yes, we can, but in this context on "context" field is interesting for
us.

I found that inside CopyErrorData() there is Assert:

Assert(CurrentMemoryContext != ErrorContext);

And in the patch we are setting using ErrorContext as short living
context,
is
it the reason of not using CopyErrorData() ?

it was not a primary reason, but sure - this routine is not designed
for direct using from elog module. Next, we can save one palloc call.

Hmm ok.

2) To reset stack to empty, FlushErrorState() can be used.

yes, it can be. I use explicit rows due different semantics, although
it does same things. FlushErrorState some global ErrorState and it is
used from other modules. I did a reset of ErrorContext. I fill a small
difference there - so FlushErrorState is not best name for my purpose.
What about modification:

static void
resetErrorStack()
{
errordata_stack_depth = -1;
recursion_depth = 0;
/* Delete all data in ErrorContext */
MemoryContextResetAndDeleteChildren(ErrorContext);
}

and then call in InvokeErrorCallbacks -- resetErrorStack()

and rewrote flushErrorState like

void FlushErrorState(void)
{
/* reset ErrorStack is enough */
resetErrorStack();
}

???

Nope. rather then that I would still prefer direct call of
FlushErrorState().

but I can live well with direct call of FlushErrorState() - it is only
minor change.

3) I was think about the usability of the feature and wondering how
about if
we add some header to the stack, so user can differentiate between
STACK
and
normal NOTICE message ?

Something like:

postgres=# select outer_outer_func(10);
NOTICE: ----- Call Stack -----
PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
outer_outer_func
------------------
20
(1 row)

I understand, but I don't think so it is good idea. Sometimes, you
would to use context for different purposes than debug log - for
example - you should to identify top most call or near call - and any
additional lines means some little bit more difficult parsing. You can
add this line simply (if you want)

RAISE NOTICE e'--- Call Stack ---\n%', stack

but there are more use cases, where you would not this information (or
you can use own header (different language)), and better returns only
clean data.

I don't know but I still feel like given header from function it self
would
be
nice to have things. Because it will help user to differentiate between
STACK and normal NOTICE context message.

I worked on point 2) and 3) and PFA patch for reference.

so

*1 CopyErrorData does one useless palloc more and it is too generic,

Ok

*2 it is possible - I have not strong opinion

Prefer FlushErrorState() call.

ok

*3 no, I would to return data in most simply and clean form without any
sugar

As a user perspective it would be nice to have sugar layer around.

I understand, but disagree - strings are simply joined and is
difficult to separate it. I have strong opinion here.

* a reason for this construct is not only using in debug notices
* sugar is not used in GET DIAGNOSTICS statement ever - so possible
introduction is not consistent with other

Hmm because sugar is not used in GET DIAGNOSTICS statement ever,
I think its should be fine.

How about point 2) To reset stack to empty, FlushErrorState() can be used. ?

I can use FlushErrorState() - final decision should do commiter.

Thank you

I'll send remastered patch today.

Regards

Pavel

Regards

Pavel

What do you think?

Others or committer having any opinion ?

???

Regards

Pavel

Thanks,
Rushabh

On Sat, Feb 2, 2013 at 2:53 PM, Pavel Stehule
<pavel.stehule@gmail.com>
wrote:

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET
DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN
outer_outer
─────────────
20
(1 row)

Ideas, comments?

Regards

Pavel Stehule

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Rushabh Lathia

--
Rushabh Lathia

--
Rushabh Lathia

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Rushabh Lathia

rushabh.lathia@gmail.com

over 12 years ago

In reply to: Pavel Stehule (#8)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

On Wed, Jun 26, 2013 at 5:11 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:

2013/6/26 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Tue, Jun 25, 2013 at 11:09 AM, Pavel Stehule <pavel.stehule@gmail.com

wrote:

2013/6/25 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule <

pavel.stehule@gmail.com>

wrote:

Hello

This is fragment ofmy old code from orafce package - it is

functional,

but it is written little bit more generic due impossible access to
static variables (in elog.c)

static char*
dbms_utility_format_call_stack(char mode)
{
MemoryContext oldcontext = CurrentMemoryContext;
ErrorData *edata;
ErrorContextCallback *econtext;
StringInfo sinfo;

#if PG_VERSION_NUM >= 80400
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO,
TEXTDOMAIN);
#else
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO);
#endif

MemoryContextSwitchTo(oldcontext);

for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext->previous)
(*econtext->callback) (econtext->arg);

edata = CopyErrorData();
FlushErrorState();

https://github.com/orafce/orafce/blob/master/utility.c

2013/6/24 Rushabh Lathia <rushabh.lathia@gmail.com>:

Hi,

Use of this feature is to get call stack for debugging without
raising
exception. This definitely seems very useful.

Here are my comments about the submitted patch:

Patch gets applied cleanly (there are couple of white space warning
but
that's
into test case file). make and make install too went smooth. make
check
was smooth too. Did some manual testing about feature and its went
smooth.

However would like to share couple of points:

My main motive is concentration to stack info string only. So I

didn't

use err_start function, and I didn't use CopyErrorData too. These
routines does lot of other work.

1) I noticed that you did the initialization of edata manually,

just

wondering
can't we directly use CopyErrorData() which will do that job ?

yes, we can, but in this context on "context" field is interesting

for

us.

I found that inside CopyErrorData() there is Assert:

Assert(CurrentMemoryContext != ErrorContext);

And in the patch we are setting using ErrorContext as short living
context,
is
it the reason of not using CopyErrorData() ?

it was not a primary reason, but sure - this routine is not designed
for direct using from elog module. Next, we can save one palloc call.

Hmm ok.

2) To reset stack to empty, FlushErrorState() can be used.

yes, it can be. I use explicit rows due different semantics, although
it does same things. FlushErrorState some global ErrorState and it is
used from other modules. I did a reset of ErrorContext. I fill a

small

difference there - so FlushErrorState is not best name for my

purpose.

What about modification:

static void
resetErrorStack()
{
errordata_stack_depth = -1;
recursion_depth = 0;
/* Delete all data in ErrorContext */
MemoryContextResetAndDeleteChildren(ErrorContext);
}

and then call in InvokeErrorCallbacks -- resetErrorStack()

and rewrote flushErrorState like

void FlushErrorState(void)
{
/* reset ErrorStack is enough */
resetErrorStack();
}

???

Nope. rather then that I would still prefer direct call of
FlushErrorState().

but I can live well with direct call of FlushErrorState() - it is

only

minor change.

3) I was think about the usability of the feature and wondering how
about if
we add some header to the stack, so user can differentiate between
STACK
and
normal NOTICE message ?

Something like:

postgres=# select outer_outer_func(10);
NOTICE: ----- Call Stack -----
PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
outer_outer_func
------------------
20
(1 row)

I understand, but I don't think so it is good idea. Sometimes, you
would to use context for different purposes than debug log - for
example - you should to identify top most call or near call - and any
additional lines means some little bit more difficult parsing. You

can

add this line simply (if you want)

RAISE NOTICE e'--- Call Stack ---\n%', stack

but there are more use cases, where you would not this information

(or

you can use own header (different language)), and better returns only
clean data.

I don't know but I still feel like given header from function it self
would
be
nice to have things. Because it will help user to differentiate

between

STACK and normal NOTICE context message.

I worked on point 2) and 3) and PFA patch for reference.

so

*1 CopyErrorData does one useless palloc more and it is too generic,

Ok

*2 it is possible - I have not strong opinion

Prefer FlushErrorState() call.

ok

*3 no, I would to return data in most simply and clean form without

any

sugar

As a user perspective it would be nice to have sugar layer around.

I understand, but disagree - strings are simply joined and is
difficult to separate it. I have strong opinion here.

* a reason for this construct is not only using in debug notices
* sugar is not used in GET DIAGNOSTICS statement ever - so possible
introduction is not consistent with other

Hmm because sugar is not used in GET DIAGNOSTICS statement ever,
I think its should be fine.

How about point 2) To reset stack to empty, FlushErrorState() can be

used. ?

I can use FlushErrorState() - final decision should do commiter.

Thank you

I'll send remastered patch today.

Thanks Pavel.

Regards

Pavel

Regards

Pavel

What do you think?

Others or committer having any opinion ?

???

Regards

Pavel

Thanks,
Rushabh

On Sat, Feb 2, 2013 at 2:53 PM, Pavel Stehule
<pavel.stehule@gmail.com>
wrote:

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET
DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN
outer_outer
─────────────
20
(1 row)

Ideas, comments?

Regards

Pavel Stehule

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org

)

To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Rushabh Lathia

--
Rushabh Lathia

--
Rushabh Lathia

--
Rushabh Lathia

#10

Pavel Stehule

pavel.stehule@gmail.com

over 12 years ago

In reply to: Rushabh Lathia (#9)

1 attachment(s)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

Hello

updated patch with some basic doc

Regards

Pavel

2013/6/26 Rushabh Lathia <rushabh.lathia@gmail.com>:

Show quoted text

On Wed, Jun 26, 2013 at 5:11 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2013/6/26 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Tue, Jun 25, 2013 at 11:09 AM, Pavel Stehule
<pavel.stehule@gmail.com>
wrote:

2013/6/25 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule
<pavel.stehule@gmail.com>
wrote:

Hello

This is fragment ofmy old code from orafce package - it is
functional,
but it is written little bit more generic due impossible access to
static variables (in elog.c)

static char*
dbms_utility_format_call_stack(char mode)
{
MemoryContext oldcontext = CurrentMemoryContext;
ErrorData *edata;
ErrorContextCallback *econtext;
StringInfo sinfo;

#if PG_VERSION_NUM >= 80400
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO,
TEXTDOMAIN);
#else
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO);
#endif

MemoryContextSwitchTo(oldcontext);

for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext->previous)
(*econtext->callback) (econtext->arg);

edata = CopyErrorData();
FlushErrorState();

https://github.com/orafce/orafce/blob/master/utility.c

2013/6/24 Rushabh Lathia <rushabh.lathia@gmail.com>:

Hi,

Use of this feature is to get call stack for debugging without
raising
exception. This definitely seems very useful.

Here are my comments about the submitted patch:

Patch gets applied cleanly (there are couple of white space
warning
but
that's
into test case file). make and make install too went smooth. make
check
was smooth too. Did some manual testing about feature and its went
smooth.

However would like to share couple of points:

My main motive is concentration to stack info string only. So I
didn't
use err_start function, and I didn't use CopyErrorData too. These
routines does lot of other work.

1) I noticed that you did the initialization of edata manually,
just
wondering
can't we directly use CopyErrorData() which will do that job ?

yes, we can, but in this context on "context" field is interesting
for
us.

I found that inside CopyErrorData() there is Assert:

Assert(CurrentMemoryContext != ErrorContext);

And in the patch we are setting using ErrorContext as short living
context,
is
it the reason of not using CopyErrorData() ?

it was not a primary reason, but sure - this routine is not designed
for direct using from elog module. Next, we can save one palloc
call.

Hmm ok.

2) To reset stack to empty, FlushErrorState() can be used.

yes, it can be. I use explicit rows due different semantics,
although
it does same things. FlushErrorState some global ErrorState and it
is
used from other modules. I did a reset of ErrorContext. I fill a
small
difference there - so FlushErrorState is not best name for my
purpose.
What about modification:

static void
resetErrorStack()
{
errordata_stack_depth = -1;
recursion_depth = 0;
/* Delete all data in ErrorContext */
MemoryContextResetAndDeleteChildren(ErrorContext);
}

and then call in InvokeErrorCallbacks -- resetErrorStack()

and rewrote flushErrorState like

void FlushErrorState(void)
{
/* reset ErrorStack is enough */
resetErrorStack();
}

???

Nope. rather then that I would still prefer direct call of
FlushErrorState().

but I can live well with direct call of FlushErrorState() - it is
only
minor change.

3) I was think about the usability of the feature and wondering
how
about if
we add some header to the stack, so user can differentiate between
STACK
and
normal NOTICE message ?

Something like:

postgres=# select outer_outer_func(10);
NOTICE: ----- Call Stack -----
PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
outer_outer_func
------------------
20
(1 row)

I understand, but I don't think so it is good idea. Sometimes, you
would to use context for different purposes than debug log - for
example - you should to identify top most call or near call - and
any
additional lines means some little bit more difficult parsing. You
can
add this line simply (if you want)

RAISE NOTICE e'--- Call Stack ---\n%', stack

but there are more use cases, where you would not this information
(or
you can use own header (different language)), and better returns
only
clean data.

I don't know but I still feel like given header from function it self
would
be
nice to have things. Because it will help user to differentiate
between
STACK and normal NOTICE context message.

I worked on point 2) and 3) and PFA patch for reference.

so

*1 CopyErrorData does one useless palloc more and it is too generic,

Ok

*2 it is possible - I have not strong opinion

Prefer FlushErrorState() call.

ok

*3 no, I would to return data in most simply and clean form without
any
sugar

As a user perspective it would be nice to have sugar layer around.

I understand, but disagree - strings are simply joined and is
difficult to separate it. I have strong opinion here.

* a reason for this construct is not only using in debug notices
* sugar is not used in GET DIAGNOSTICS statement ever - so possible
introduction is not consistent with other

Hmm because sugar is not used in GET DIAGNOSTICS statement ever,
I think its should be fine.

How about point 2) To reset stack to empty, FlushErrorState() can be
used. ?

I can use FlushErrorState() - final decision should do commiter.

Thank you

I'll send remastered patch today.

Thanks Pavel.

Regards

Pavel

Regards

Pavel

What do you think?

Others or committer having any opinion ?

???

Regards

Pavel

Thanks,
Rushabh

On Sat, Feb 2, 2013 at 2:53 PM, Pavel Stehule
<pavel.stehule@gmail.com>
wrote:

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack
for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used
four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET
DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN
outer_outer
─────────────
20
(1 row)

Ideas, comments?

Regards

Pavel Stehule

--
Sent via pgsql-hackers mailing list
(pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Rushabh Lathia

--
Rushabh Lathia

--
Rushabh Lathia

--
Rushabh Lathia

Attachments:

get_diagnostics_context.patchapplication/octet-stream; name=get_diagnostics_context.patchDownload

*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 2712,2717 **** END;
--- 2712,2765 ----
      </para>
     </sect3>
    </sect2>
+ 
+   <sect2 id="plpgsql-get-diagnostics-context">
+    <title>Obtaining information about an calls stack</title>
+ 
+    <para>
+ 
+ <synopsis>
+ GET <optional> CURRENT </optional> DIAGNOSTICS <replaceable>variable</replaceable> = <replaceable>PG_CONTEXT</replaceable> <optional> , ... </optional>;
+ </synopsis>
+ 
+ 
+     With command <command>GET DIAGNOSTICS</command> and status
+     item <varname>PG_CONTEXT</> is possible to get line(s) of
+     text describing the call stack. A first row is related to
+     current function and currently executed <command>GET DIAGNOSTICS</command>
+     command. Second row is related to first outer function,
+     and related statement, etc.
+ 
+ <programlisting>
+ CREATE OR REPLACE FUNCTION public.outer_func() RETURNS integer AS $$
+ BEGIN
+   RETURN inner_func();
+ END;
+ $$ LANGUAGE plpgsql;
+ 
+ CREATE OR REPLACE FUNCTION public.inner_func() RETURNS integer AS  $$
+ DECLARE
+   stack text;
+ BEGIN
+   GET DIAGNOSTICS stack = PG_CONTEXT;
+   RAISE NOTICE e'--- Call Stack ---\n%', stack;
+   RETURN 1;
+ END;
+ $$ LANGUAGE plpgsql;
+ 
+ SELECT outer_func();
+ 
+ NOTICE:  --- Call Stack ---
+ PL/pgSQL function inner_func() line 4 at GET DIAGNOSTICS
+ PL/pgSQL function outer_func() line 3 at RETURN
+  outer_func
+  ------------
+            1
+ (1 row)
+ </programlisting>
+ 
+    </para>
+   </sect2>
    </sect1>
  
    <sect1 id="plpgsql-cursors">
*** a/src/backend/utils/error/elog.c
--- b/src/backend/utils/error/elog.c
***************
*** 3120,3122 **** trace_recovery(int trace_level)
--- 3120,3168 ----
  
  	return trace_level;
  }
+ 
+ /*
+  * InvokeErrorCallbacks - invoke registrated error callback functions.
+  */
+ char *
+ InvokeErrorCallbacks(void)
+ {
+ 	ErrorData  *edata;
+ 	ErrorContextCallback *econtext;
+ 	MemoryContext oldcontext = CurrentMemoryContext;
+ 	char		*result;
+ 
+ 	/* this function should not be started in exception handler */
+ 	Assert(recursion_depth == 0);
+ 
+ 	if (++errordata_stack_depth >= ERRORDATA_STACK_SIZE)
+ 	{
+ 		errordata_stack_depth = -1;		/* make room on stack */
+ 		ereport(PANIC, (errmsg_internal("ERRORDATA_STACK_SIZE exceeded")));
+ 	}
+ 
+ 	/* Initialize data for this error frame */
+ 	edata = &errordata[errordata_stack_depth];
+ 	MemSet(edata, 0, sizeof(ErrorData));
+ 
+ 	/* Use ErrorContext as short living context */
+ 	MemoryContextSwitchTo(ErrorContext);
+ 
+ 	/*
+ 	 * Call any context callback functions.  Errors occurring in callback
+ 	 * functions will be treated as recursive errors --- this ensures we will
+ 	 * avoid infinite recursion (see errstart).
+ 	 */
+ 	for (econtext = error_context_stack;
+ 		 econtext != NULL;
+ 		 econtext = econtext->previous)
+ 		(*econtext->callback) (econtext->arg);
+ 
+ 	MemoryContextSwitchTo(oldcontext);
+ 	result = pstrdup(edata->context ? edata->context : "");
+ 
+ 	/* Reset error stack */
+ 	FlushErrorState();
+ 
+ 	return result;
+ }
*** a/src/include/utils/elog.h
--- b/src/include/utils/elog.h
***************
*** 406,411 **** extern void FlushErrorState(void);
--- 406,413 ----
  extern void ReThrowError(ErrorData *edata) __attribute__((noreturn));
  extern void pg_re_throw(void) __attribute__((noreturn));
  
+ extern char *InvokeErrorCallbacks(void);
+ 
  /* Hook for intercepting messages before they are sent to the server log */
  typedef void (*emit_log_hook_type) (ErrorData *edata);
  extern PGDLLIMPORT emit_log_hook_type emit_log_hook;
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***************
*** 1574,1579 **** exec_stmt_getdiag(PLpgSQL_execstate *estate, PLpgSQL_stmt_getdiag *stmt)
--- 1574,1591 ----
  									 estate->cur_error->message);
  				break;
  
+ 			case PLPGSQL_GETDIAG_CONTEXT:
+ 				{
+ 					char *context;
+ 
+ 					context = InvokeErrorCallbacks();
+ 
+ 					exec_assign_c_string(estate, var,
+ 										context);
+ 					pfree(context);
+ 				}
+ 				break;
+ 
  			default:
  				elog(ERROR, "unrecognized diagnostic item kind: %d",
  					 diag_item->kind);
*** a/src/pl/plpgsql/src/pl_funcs.c
--- b/src/pl/plpgsql/src/pl_funcs.c
***************
*** 273,278 **** plpgsql_getdiag_kindname(int kind)
--- 273,280 ----
  {
  	switch (kind)
  	{
+ 		case PLPGSQL_GETDIAG_CONTEXT:
+ 			return "PG_CONTEXT";
  		case PLPGSQL_GETDIAG_ROW_COUNT:
  			return "ROW_COUNT";
  		case PLPGSQL_GETDIAG_RESULT_OID:
*** a/src/pl/plpgsql/src/pl_gram.y
--- b/src/pl/plpgsql/src/pl_gram.y
***************
*** 298,303 **** static	List			*read_raise_options(void);
--- 298,304 ----
  %token <keyword>	K_OPTION
  %token <keyword>	K_OR
  %token <keyword>	K_PERFORM
+ %token <keyword>	K_PG_CONTEXT
  %token <keyword>	K_PG_EXCEPTION_CONTEXT
  %token <keyword>	K_PG_EXCEPTION_DETAIL
  %token <keyword>	K_PG_EXCEPTION_HINT
***************
*** 884,889 **** stmt_getdiag	: K_GET getdiag_area_opt K_DIAGNOSTICS getdiag_list ';'
--- 885,891 ----
  								/* these fields are disallowed in stacked case */
  								case PLPGSQL_GETDIAG_ROW_COUNT:
  								case PLPGSQL_GETDIAG_RESULT_OID:
+ 								case PLPGSQL_GETDIAG_CONTEXT:
  									if (new->is_stacked)
  										ereport(ERROR,
  												(errcode(ERRCODE_SYNTAX_ERROR),
***************
*** 956,961 **** getdiag_item :
--- 958,966 ----
  						int	tok = yylex();
  
  						if (tok_is_keyword(tok, &yylval,
+ 										   K_PG_CONTEXT, "pg_context"))
+ 							$$ = PLPGSQL_GETDIAG_CONTEXT;
+ 						else if (tok_is_keyword(tok, &yylval,
  										   K_ROW_COUNT, "row_count"))
  							$$ = PLPGSQL_GETDIAG_ROW_COUNT;
  						else if (tok_is_keyword(tok, &yylval,
***************
*** 2252,2257 **** unreserved_keyword	:
--- 2257,2263 ----
  				| K_NO
  				| K_NOTICE
  				| K_OPTION
+ 				| K_PG_CONTEXT
  				| K_PG_EXCEPTION_CONTEXT
  				| K_PG_EXCEPTION_DETAIL
  				| K_PG_EXCEPTION_HINT
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
***************
*** 130,135 **** static const ScanKeyword unreserved_keywords[] = {
--- 130,136 ----
  	PG_KEYWORD("no", K_NO, UNRESERVED_KEYWORD)
  	PG_KEYWORD("notice", K_NOTICE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("option", K_OPTION, UNRESERVED_KEYWORD)
+ 	PG_KEYWORD("pg_context", K_PG_CONTEXT, UNRESERVED_KEYWORD)
  	PG_KEYWORD("pg_exception_context", K_PG_EXCEPTION_CONTEXT, UNRESERVED_KEYWORD)
  	PG_KEYWORD("pg_exception_detail", K_PG_EXCEPTION_DETAIL, UNRESERVED_KEYWORD)
  	PG_KEYWORD("pg_exception_hint", K_PG_EXCEPTION_HINT, UNRESERVED_KEYWORD)
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 122,127 **** enum
--- 122,128 ----
   */
  enum
  {
+ 	PLPGSQL_GETDIAG_CONTEXT,
  	PLPGSQL_GETDIAG_ROW_COUNT,
  	PLPGSQL_GETDIAG_RESULT_OID,
  	PLPGSQL_GETDIAG_ERROR_CONTEXT,
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
***************
*** 4863,4865 **** ERROR:  value for domain orderedarray violates check constraint "sorted"
--- 4863,4913 ----
  CONTEXT:  PL/pgSQL function testoa(integer,integer,integer) line 5 at assignment
  drop function arrayassign1();
  drop function testoa(x1 int, x2 int, x3 int);
+ -- access to call stack
+ create function inner_func(int)
+ returns int as $$
+ declare _context text;
+ begin
+   get diagnostics _context = pg_context;
+   raise notice '***%***', _context;
+   return 2 * $1;
+ end;
+ $$ language plpgsql;
+ create or replace function outer_func(int)
+ returns int as $$
+ begin
+   return inner_func($1);
+ end;
+ $$ language plpgsql;
+ create or replace function outer_outer_func(int)
+ returns int as $$
+ begin
+   return outer_func($1);
+ end;
+ $$ language plpgsql;
+ select outer_outer_func(10);
+ NOTICE:  ***PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
+ PL/pgSQL function outer_func(integer) line 3 at RETURN
+ PL/pgSQL function outer_outer_func(integer) line 3 at RETURN***
+ CONTEXT:  PL/pgSQL function outer_func(integer) line 3 at RETURN
+ PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
+  outer_outer_func 
+ ------------------
+                20
+ (1 row)
+ 
+ -- repeated call should to work
+ select outer_outer_func(20);
+ NOTICE:  ***PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
+ PL/pgSQL function outer_func(integer) line 3 at RETURN
+ PL/pgSQL function outer_outer_func(integer) line 3 at RETURN***
+ CONTEXT:  PL/pgSQL function outer_func(integer) line 3 at RETURN
+ PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
+  outer_outer_func 
+ ------------------
+                40
+ (1 row)
+ 
+ drop function outer_outer_func(int);
+ drop function outer_func(int);
+ drop function inner_func(int);
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
***************
*** 3848,3850 **** select testoa(1,2,1); -- fail at update
--- 3848,3883 ----
  
  drop function arrayassign1();
  drop function testoa(x1 int, x2 int, x3 int);
+ 
+ -- access to call stack
+ create function inner_func(int)
+ returns int as $$
+ declare _context text;
+ begin
+   get diagnostics _context = pg_context;
+   raise notice '***%***', _context;
+   return 2 * $1;
+ end;
+ $$ language plpgsql;
+ 
+ create or replace function outer_func(int)
+ returns int as $$
+ begin
+   return inner_func($1);
+ end;
+ $$ language plpgsql;
+ 
+ create or replace function outer_outer_func(int)
+ returns int as $$
+ begin
+   return outer_func($1);
+ end;
+ $$ language plpgsql;
+ 
+ select outer_outer_func(10);
+ -- repeated call should to work
+ select outer_outer_func(20);
+ 
+ drop function outer_outer_func(int);
+ drop function outer_func(int);
+ drop function inner_func(int);

#11

Rushabh Lathia

rushabh.lathia@gmail.com

over 12 years ago

In reply to: Pavel Stehule (#10)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

Latest patch looks good to me.

Regards,
Rushabh

On Wed, Jun 26, 2013 at 11:21 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:

Hello

updated patch with some basic doc

Regards

Pavel

2013/6/26 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Wed, Jun 26, 2013 at 5:11 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2013/6/26 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Tue, Jun 25, 2013 at 11:09 AM, Pavel Stehule
<pavel.stehule@gmail.com>
wrote:

2013/6/25 Rushabh Lathia <rushabh.lathia@gmail.com>:

On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule
<pavel.stehule@gmail.com>
wrote:

Hello

This is fragment ofmy old code from orafce package - it is
functional,
but it is written little bit more generic due impossible access to
static variables (in elog.c)

static char*
dbms_utility_format_call_stack(char mode)
{
MemoryContext oldcontext = CurrentMemoryContext;
ErrorData *edata;
ErrorContextCallback *econtext;
StringInfo sinfo;

#if PG_VERSION_NUM >= 80400
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO,
TEXTDOMAIN);
#else
errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO);
#endif

MemoryContextSwitchTo(oldcontext);

for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext->previous)
(*econtext->callback) (econtext->arg);

edata = CopyErrorData();
FlushErrorState();

https://github.com/orafce/orafce/blob/master/utility.c

2013/6/24 Rushabh Lathia <rushabh.lathia@gmail.com>:

Hi,

Use of this feature is to get call stack for debugging without
raising
exception. This definitely seems very useful.

Here are my comments about the submitted patch:

Patch gets applied cleanly (there are couple of white space
warning
but
that's
into test case file). make and make install too went smooth.

make

check
was smooth too. Did some manual testing about feature and its

went

smooth.

However would like to share couple of points:

My main motive is concentration to stack info string only. So I
didn't
use err_start function, and I didn't use CopyErrorData too. These
routines does lot of other work.

1) I noticed that you did the initialization of edata manually,
just
wondering
can't we directly use CopyErrorData() which will do that job ?

yes, we can, but in this context on "context" field is interesting
for
us.

I found that inside CopyErrorData() there is Assert:

Assert(CurrentMemoryContext != ErrorContext);

And in the patch we are setting using ErrorContext as short

living

context,
is
it the reason of not using CopyErrorData() ?

it was not a primary reason, but sure - this routine is not

designed

for direct using from elog module. Next, we can save one palloc
call.

Hmm ok.

2) To reset stack to empty, FlushErrorState() can be used.

yes, it can be. I use explicit rows due different semantics,
although
it does same things. FlushErrorState some global ErrorState and it
is
used from other modules. I did a reset of ErrorContext. I fill a
small
difference there - so FlushErrorState is not best name for my
purpose.
What about modification:

static void
resetErrorStack()
{
errordata_stack_depth = -1;
recursion_depth = 0;
/* Delete all data in ErrorContext */
MemoryContextResetAndDeleteChildren(ErrorContext);
}

and then call in InvokeErrorCallbacks -- resetErrorStack()

and rewrote flushErrorState like

void FlushErrorState(void)
{
/* reset ErrorStack is enough */
resetErrorStack();
}

???

Nope. rather then that I would still prefer direct call of
FlushErrorState().

but I can live well with direct call of FlushErrorState() - it is
only
minor change.

3) I was think about the usability of the feature and wondering
how
about if
we add some header to the stack, so user can differentiate

between

STACK
and
normal NOTICE message ?

Something like:

postgres=# select outer_outer_func(10);
NOTICE: ----- Call Stack -----
PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
outer_outer_func
------------------
20
(1 row)

I understand, but I don't think so it is good idea. Sometimes, you
would to use context for different purposes than debug log - for
example - you should to identify top most call or near call - and
any
additional lines means some little bit more difficult parsing. You
can
add this line simply (if you want)

RAISE NOTICE e'--- Call Stack ---\n%', stack

but there are more use cases, where you would not this information
(or
you can use own header (different language)), and better returns
only
clean data.

I don't know but I still feel like given header from function it

self

would
be
nice to have things. Because it will help user to differentiate
between
STACK and normal NOTICE context message.

I worked on point 2) and 3) and PFA patch for reference.

so

*1 CopyErrorData does one useless palloc more and it is too

generic,

Ok

*2 it is possible - I have not strong opinion

Prefer FlushErrorState() call.

ok

*3 no, I would to return data in most simply and clean form

without

any
sugar

As a user perspective it would be nice to have sugar layer around.

I understand, but disagree - strings are simply joined and is
difficult to separate it. I have strong opinion here.

* a reason for this construct is not only using in debug notices
* sugar is not used in GET DIAGNOSTICS statement ever - so possible
introduction is not consistent with other

Hmm because sugar is not used in GET DIAGNOSTICS statement ever,
I think its should be fine.

How about point 2) To reset stack to empty, FlushErrorState() can be
used. ?

I can use FlushErrorState() - final decision should do commiter.

Thank you

I'll send remastered patch today.

Thanks Pavel.

Regards

Pavel

Regards

Pavel

What do you think?

Others or committer having any opinion ?

???

Regards

Pavel

Thanks,
Rushabh

On Sat, Feb 2, 2013 at 2:53 PM, Pavel Stehule
<pavel.stehule@gmail.com>
wrote:

Hello

I propose enhancing GET DIAGNOSTICS statement about new field
PG_CONTEXT. It is similar to GET STACKED DIAGNOSTICS'
PG_EXCEPTION_CONTEXT.

Motivation for this proposal is possibility to get call stack
for
debugging without raising exception.

This code is based on cleaned code from Orafce, where is used
four
years without any error reports.

CREATE OR REPLACE FUNCTION public."inner"(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
return 2 * $1;
end;
$function$

postgres=# select outer_outer(10);
NOTICE: ***PL/pgSQL function "inner"(integer) line 4 at GET
DIAGNOSTICS
PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function "outer"(integer) line 3 at RETURN
PL/pgSQL function outer_outer(integer) line 3 at RETURN
outer_outer
─────────────
20
(1 row)

Ideas, comments?

Regards

Pavel Stehule

--
Sent via pgsql-hackers mailing list
(pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Rushabh Lathia

--
Rushabh Lathia

--
Rushabh Lathia

--
Rushabh Lathia

--
Rushabh Lathia

#12

Stephen Frost

sfrost@snowman.net

over 12 years ago

In reply to: Rushabh Lathia (#11)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

All,

* Rushabh Lathia (rushabh.lathia@gmail.com) wrote:

Latest patch looks good to me.

I've committed this, with some pretty serious clean-up. In the future,
please don't simply copy/paste code w/o any updates to the comments
included, particularly when the comments no longer make sense in the new
place.

Thanks,

Stephen

#13

Pavel Stehule

pavel.stehule@gmail.com

over 12 years ago

In reply to: Stephen Frost (#12)

Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement

Hello

2013/7/25 Stephen Frost <sfrost@snowman.net>:

All,

* Rushabh Lathia (rushabh.lathia@gmail.com) wrote:

Latest patch looks good to me.

I've committed this, with some pretty serious clean-up. In the future,
please don't simply copy/paste code w/o any updates to the comments
included, particularly when the comments no longer make sense in the new
place.

Thank you very much

Regards

Pavel

Thanks,

Stephen

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers