Message-ID: <974c2531-536a-4223-a264-166872386459@iki.fi>
Date: Thu, 4 Jul 2024 18:27:43 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Improving PL/Tcl's error context reports
To: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@lists.postgresql.org
References: <890581.1717609350@sss.pgh.pa.us>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <890581.1717609350@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/974c2531-536a-4223-a264-166872386459%40iki.fi>
Precedence: bulk

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.

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)