Date: Mon, 3 Jun 2024 18:16:48 +0200
From: Erik Wienhold <ewie@ewie.name>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pierre Forstmann <pierre.forstmann@gmail.com>, 
	"a.kozhemyakin" <a.kozhemyakin@postgrespro.ru>, pgsql-hackers@lists.postgresql.org
Subject: Re: pltcl crashes due to a syntax error
Message-ID: <309214c5-d4da-4e00-938d-0e35990e564c@ewie.name>
References: <6a2a1c40-2b2c-4a33-8b72-243c0766fcda@postgrespro.ru>
 <CAM-sOH9zC5VeycS0BFCV4CAEfDL4OLEh_41oWtVLfbOZCtxNFw@mail.gmail.com>
 <5af13487-7722-4bdd-bad3-939598c8dd4e@ewie.name>
 <2365308.1717366548@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2365308.1717366548@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/309214c5-d4da-4e00-938d-0e35990e564c%40ewie.name>
Precedence: bulk

On 2024-06-03 00:15 +0200, Tom Lane wrote:
> The new bit of information that this bug report provides is that it's
> possible to get a TCL_ERROR result without Tcl having set errorInfo.
> That seems a tad odd, and it must happen only in weird corner cases,
> else we'd have heard of this decades ago.  Not sure if it's worth
> trying to characterize those cases further, however.

ISTM that errorInfo is set automatically only during script evaluation.
The Tcl_AddErrorInfo manpage says:

"The -errorinfo option value is gradually built up as an error unwinds
 through the nested operations.  Each time an error code is returned to
 Tcl_Eval, or any of the routines that performs script evaluation, the
 procedure Tcl_AddErrorInfo is called to add additional text to the
 -errorinfo value describing the command that was being executed when
 the error occurred.  By the time the error has been passed all the way
 back to the application, it will contain a complete trace of the
 activity in progress when the error occurred."

Tcl 8.4 basically uses the same wording.

Except for the reported case, we only call throw_tcl_error in three
places, all after checking the return code from Tcl_EvalObjEx.  And this
one Tcl_ListObjGetElements instance is not called during script
evaluation.

> > Or just do away with throw_tcl_error and call ereport directly.
> 
> I'd say this adds to the importance of having throw_tcl_error,
> because now it's even more complex than before, and there are
> multiple call sites.

I agree to have some uniform error handling.  But from the current usage
it looks as if throw_tcl_error is tied to Tcl_EvalObjEx.

-- 
Erik