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
