Re: Improving PL/Tcl's error context reports

čt 4. 7. 2024 v 17:27 odesílatel Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
napsal:

> On 05/06/2024 20:42, Tom Lane wrote:
> > While working on commit b631d0149, I got a bee in my bonnet about
> > how unfriendly PL/Tcl's error CONTEXT reports are:
> >
> > * The context reports expose PL/Tcl's internal names for the Tcl
> > procedures it creates, which'd be fine if those names were readable.
> > But actually they're something like "__PLTcl_proc_NNNN", where NNNN
> > is the function OID. Not only is that unintelligible, but because
> > the OIDs aren't stable this forces us to disable display of the
> > CONTEXT lines in all of PL/Tcl's regression tests.
> >
> > * The first line of the context report (almost?) always duplicates
> > the primary error message, which is redundant and not per our
> > normal reporting style.
> >
> > So attached is a patch that attempts to improve this situation.
> >
> > The key question is how to avoid including function OIDs in the
> > strings that will appear in the regression test outputs. The
> > answer I propose is to start with an internal name like
> > "__PLTcl_proc_NAME", where NAME is the function's normal SQL name,
> > and then append the OID only if that function name is not unique.
> > As long as we don't create test cases that involve throwing
> > errors from duplicatively-named functions, we can show the context
> > reports and still have stable regression outputs. I think this will
> > improve the user experience for regular users too.
>
> Yes, that sounds a lot nicer.
>
> What happens if you rename a function? I guess the error context will
> still print the old name, but that's pretty harmless.
>

The rename should to generate different tid, so the function will be
recompiled

<-->/************************************************************
<--> * If it's present, must check whether it's still up to date.
<--> * This is needed because CREATE OR REPLACE FUNCTION can modify the
<--> * function's pg_proc entry without changing its OID.
<--> ************************************************************/
<-->if (prodesc != NULL &&
<--><-->prodesc->internal_proname != NULL &&
<--><-->prodesc->fn_xmin == HeapTupleHeaderGetRawXmin(procTup->t_data) &&
<--><-->ItemPointerEquals(&prodesc->fn_tid, &procTup->t_self))
<-->{
<--><-->/* It's still up-to-date, so we can use it */
<--><-->ReleaseSysCache(procTup);
<--><-->return prodesc;
<-->}

>
> Hmm, could we do something with tcl namespaces to allow having two
> procedures with the same name? E.g. create a separate namespace, based
> on the OID, for each procedure. I wonder how the stack trace would look
> like then.
>
> --
> Heikki Linnakangas
> Neon (https://neon.tech)
>
>
>
>