Thread: proposal: better support for debugging of overloaded functions
Hello
I am missing a some unique identifier in exception info. I would to
use it in combination with \sf statement
I have a log
WARNING: NP_CPS: a cannot to build a RSLT object
DETAIL: dsql_text: SELECT * FROM
public._npacceptflatfile(order_nr:=to_number('O00000032',
'O99999999')::int,sequence_nr:=1,ref_sequence_nr:=2,recipient_op:=201,losing_op:=303)
message: cannot to identify real type for record type variable
CONTEXT: PL/pgSQL function "assign_rslts" line 50 at assignment
SQL statement "SELECT workflow.assign_rslts('2011-12-18', '09:30:30',
to_operator := 201, from_operator := 303)"
PL/pgSQL function "inline_code_block" line 855 at PERFORM
and I would to look on "assign_rslts" function, but
ohs=# \sf workflow.assign_rslts
ERROR: more than one function named "workflow.assign_rslts"
and I have to find a parameters and manually build a parameters list.
My proposal is enhancing l CONTEXT line about function's oid and
possibility to use this oid in \sf and \df function
some like
CONTEXT: PL/pgSQL function "assign_rslts" line 50 at assignment (oid: 65903)
...
\sf+ 65903
This simple feature can reduce a time that is necessary to identify a
bug in overloaded functions.
Other possibility is just enhancing context line to be more friendly
to \sf statement
CONTEXT: PL/pgSQL function workflow.assign_rslts(date,time without
time zone,operatorid_type,operatorid_type)"" line 50 at assignment
But this is not too readable and it implementation is harder, because
in exception time is not access to system tables - so this string
should be cached somewhere.
Notes, ideas??
Regards
Pavel Stehule
On Fri, Nov 18, 2011 at 6:24 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > CONTEXT: PL/pgSQL function "assign_rslts" line 50 at assignment (oid: 65903) > > \sf+ 65903 I'm pretty unenthused by the idea of making OIDs more user-visible than they already are. If the message is ambiguous, we should include argument types and (if not the object that would be visible under the current search_path) a schema qualification. Spitting out a five (or six or seven or eight) digit number doesn't seem like a usability improvement. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
2011/11/18 Robert Haas <robertmhaas@gmail.com>: > On Fri, Nov 18, 2011 at 6:24 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> CONTEXT: PL/pgSQL function "assign_rslts" line 50 at assignment (oid: 65903) >> >> \sf+ 65903 > > I'm pretty unenthused by the idea of making OIDs more user-visible > than they already are. If the message is ambiguous, we should include > argument types and (if not the object that would be visible under the > current search_path) a schema qualification. Spitting out a five (or > six or seven or eight) digit number doesn't seem like a usability > improvement. > yes - it's not nice - but it is simple and robust and doesn't depend on actual search_path setting. Nicer solution is a function signature - it can be assembled when function is compiled. I see only one disadvantage - signature can be too wide and can depend on search_path (and search_path can be different when function is executed and when someone run sql console). Signature should be prepared before execution, because there are no access to system tables after exception. I like any solution, because debugging of overloaded function is terrible now. Regards Pavel > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
2011/11/18 Robert Haas <robertmhaas@gmail.com>: > On Fri, Nov 18, 2011 at 6:24 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> CONTEXT: PL/pgSQL function "assign_rslts" line 50 at assignment (oid: 65903) >> >> \sf+ 65903 > > I'm pretty unenthused by the idea of making OIDs more user-visible > than they already are. If the message is ambiguous, we should include > argument types and (if not the object that would be visible under the > current search_path) a schema qualification. Spitting out a five (or > six or seven or eight) digit number doesn't seem like a usability > improvement. Is possible to add GUC variable plpgsql.log_function_signature (maybe just log_function_signature (for all PL))? I am not sure about GUC name. When this variable is true, then CONTEXT line will contain a qualified function's signature instead function name I don't would to check if function name is ambiguous or not after exception is raised. There is a problem with access to system tables and then exception handling can be slower. Using a qualified name is necessary, because psql meta statements are not "smart" - they are based on search_path and fact, so name is not ambiguous doesn't help there. Regards Pavel Stehule p.s. Other issue is missing CONTEXT line for RAISE EXCEPTION > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
On Sun, Nov 20, 2011 at 6:16 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > Is possible to add GUC variable plpgsql.log_function_signature (maybe > just log_function_signature (for all PL))? I am not sure about GUC > name. > > When this variable is true, then CONTEXT line will contain a qualified > function's signature instead function name Sure, but why? If it's possible to do that, I think we should just do it always. It might be a net reduction in readability for people who don't use overloading but do have functions with very long names and lots and lots of arguments, but even if you think that's good design, I think the general principle that an error message should uniquely identify the object responsible for the error ought to take precedence. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hello 2011/11/21 Robert Haas <robertmhaas@gmail.com>: > On Sun, Nov 20, 2011 at 6:16 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> Is possible to add GUC variable plpgsql.log_function_signature (maybe >> just log_function_signature (for all PL))? I am not sure about GUC >> name. >> >> When this variable is true, then CONTEXT line will contain a qualified >> function's signature instead function name > > Sure, but why? If it's possible to do that, I think we should just do > it always. It might be a net reduction in readability for people who > don't use overloading but do have functions with very long names and > lots and lots of arguments, but even if you think that's good design, > I think the general principle that an error message should uniquely > identify the object responsible for the error ought to take > precedence. I inclined so this is good solution there is a VIP patch patch is relative long, but almost all are changes in regress tests. Changes in plpgsql are 5 lines Regards Pavel > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
Attachment
At 2011-11-24 17:44:16 +0100, pavel.stehule@gmail.com wrote: > > patch is relative long, but almost all are changes in regress tests. > Changes in plpgsql are 5 lines The change looks good in principle. The patch applies to HEAD with a bit of fuzz and builds fine… but it fails tests, because it's incomplete. Pavel, your patch doesn't contain any changes to pl_exec.c. Did you just forget to submit them? Anyway, some errcontext() calls need to be taught to print ->fn_signature rather than ->fn_name. I made those changes, and found some more failing tests. Updated patch attached. Ready for committer. -- ams
Attachment
Hello 2012/1/26 Abhijit Menon-Sen <ams@toroid.org>: > At 2011-11-24 17:44:16 +0100, pavel.stehule@gmail.com wrote: >> >> patch is relative long, but almost all are changes in regress tests. >> Changes in plpgsql are 5 lines > > The change looks good in principle. The patch applies to HEAD with a bit > of fuzz and builds fine… but it fails tests, because it's incomplete. > > Pavel, your patch doesn't contain any changes to pl_exec.c. Did you just > forget to submit them? Anyway, some errcontext() calls need to be taught > to print ->fn_signature rather than ->fn_name. I made those changes, and > found some more failing tests. It was my mistake - using fn_signature for runtime errors is good idea > > Updated patch attached. Ready for committer. I found a small issue - there was uninitialized fn_signature for online blocks so I append line function->fn_signature = pstrdup(func_name); to plpgsql_compile_inline(char *proc_source) function modified patch is in attachment Pavel > > -- ams
Attachment
On 27.01.2012 15:48, Pavel Stehule wrote: > 2012/1/26 Abhijit Menon-Sen<ams@toroid.org>: >> Updated patch attached. Ready for committer. > > I found a small issue - there was uninitialized fn_signature for > online blocks so I append line > > function->fn_signature = pstrdup(func_name); to > plpgsql_compile_inline(char *proc_source) function > > modified patch is in attachment Thanks, committed! -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com