Re: [HACKERS] 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.steh...@gmail.comwrote:
Hello
updated patch with some basic doc
Regards
Pavel
2013/6/26 Rushabh Lathia rushabh.lat...@gmail.com:
On Wed, Jun 26, 2013 at 5:11 PM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2013/6/26 Rushabh Lathia rushabh.lat...@gmail.com:
On Tue, Jun 25, 2013 at 11:09 AM, Pavel Stehule
pavel.steh...@gmail.com
wrote:
2013/6/25 Rushabh Lathia rushabh.lat...@gmail.com:
On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule
pavel.steh...@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.lat...@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
Re: [HACKERS] 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.steh...@gmail.comwrote:
2013/6/25 Rushabh Lathia rushabh.lat...@gmail.com:
On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule pavel.steh...@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.lat...@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
Re: [HACKERS] proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement
2013/6/26 Rushabh Lathia rushabh.lat...@gmail.com:
On Tue, Jun 25, 2013 at 11:09 AM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2013/6/25 Rushabh Lathia rushabh.lat...@gmail.com:
On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule pavel.steh...@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.lat...@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
Re: [HACKERS] 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.steh...@gmail.comwrote:
2013/6/26 Rushabh Lathia rushabh.lat...@gmail.com:
On Tue, Jun 25, 2013 at 11:09 AM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2013/6/25 Rushabh Lathia rushabh.lat...@gmail.com:
On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule
pavel.steh...@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.lat...@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
Re: [HACKERS] 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.lat...@gmail.com:
On Wed, Jun 26, 2013 at 5:11 PM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2013/6/26 Rushabh Lathia rushabh.lat...@gmail.com:
On Tue, Jun 25, 2013 at 11:09 AM, Pavel Stehule
pavel.steh...@gmail.com
wrote:
2013/6/25 Rushabh Lathia rushabh.lat...@gmail.com:
On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule
pavel.steh...@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.lat...@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)
Re: [HACKERS] 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.steh...@gmail.comwrote: 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 get_diagnostics_context_initial_v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 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.lat...@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.
so
*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.steh...@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$
Re: [HACKERS] 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.steh...@gmail.comwrote:
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.lat...@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.
Re: [HACKERS] proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement
2013/6/25 Rushabh Lathia rushabh.lat...@gmail.com:
On Tue, Jun 25, 2013 at 1:47 AM, Pavel Stehule pavel.steh...@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.lat...@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
Re: [HACKERS] 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
[HACKERS] proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement
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 get_diagnostics_context_initial.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
