Thread: identifying unrecognized node type errors
As I was tracking down some of these errors in the sql/json patches I noticed that we have a whole lot of them in the code, so working out which one has triggered an error is not as easy as it might be. ISTM we could usefully prefix each such message with the name of the function in which it occurs, along the lines of this fragment for nodeFuncs.c. Thoughts? cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Attachment
On Fri, 25 Mar 2022 at 08:53, Andrew Dunstan <andrew@dunslane.net> wrote: > As I was tracking down some of these errors in the sql/json patches I > noticed that we have a whole lot of them in the code, so working out > which one has triggered an error is not as easy as it might be. ISTM we > could usefully prefix each such message with the name of the function in > which it occurs, along the lines of this fragment for nodeFuncs.c. Thoughts? Can you not use \set VERBOSITY verbose ? postgres=# \set VERBOSITY verbose postgres=# select 1/0; ERROR: 22012: division by zero LOCATION: int4div, int.c:846 David
Andrew Dunstan <andrew@dunslane.net> writes:
> As I was tracking down some of these errors in the sql/json patches I
> noticed that we have a whole lot of them in the code, so working out
> which one has triggered an error is not as easy as it might be. ISTM we
> could usefully prefix each such message with the name of the function in
> which it occurs, along the lines of this fragment for nodeFuncs.c. Thoughts?
-1. You're reinventing the error location support that already exists
inside elog. Just turn up log_error_verbosity instead.
regards, tom lane
On 3/24/22 16:10, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> As I was tracking down some of these errors in the sql/json patches I >> noticed that we have a whole lot of them in the code, so working out >> which one has triggered an error is not as easy as it might be. ISTM we >> could usefully prefix each such message with the name of the function in >> which it occurs, along the lines of this fragment for nodeFuncs.c. Thoughts? > -1. You're reinventing the error location support that already exists > inside elog. Just turn up log_error_verbosity instead. > > Yeah, must have had some brain fade. Sorry for the noise. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
All these functions are too low level to be helpful to know. Knowing the caller might actually give a hint as to where the unknown node originated from. We may get that from the stack trace if that's available. But if we could annotate the error with error_context that will be super helpful. For example, if we could annotate the error message like "while searching for columns to hash" for expression_tree_walker() called from find_hash_columns->find_cols->find_cols_walker->expression_tree_walker(). That helps to focus on group by colum expression for example. We could do that by setting up an error context in find_hash_columns(). But that's a lot of work. On Fri, Mar 25, 2022 at 2:04 AM Andrew Dunstan <andrew@dunslane.net> wrote: > > > On 3/24/22 16:10, Tom Lane wrote: > > Andrew Dunstan <andrew@dunslane.net> writes: > >> As I was tracking down some of these errors in the sql/json patches I > >> noticed that we have a whole lot of them in the code, so working out > >> which one has triggered an error is not as easy as it might be. ISTM we > >> could usefully prefix each such message with the name of the function in > >> which it occurs, along the lines of this fragment for nodeFuncs.c. Thoughts? > > -1. You're reinventing the error location support that already exists > > inside elog. Just turn up log_error_verbosity instead. > > > > > > > > Yeah, must have had some brain fade. Sorry for the noise. > > > cheers > > > andrew > > -- > Andrew Dunstan > EDB: https://www.enterprisedb.com > > > -- Best Wishes, Ashutosh Bapat
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:
> All these functions are too low level to be helpful to know. Knowing
> the caller might actually give a hint as to where the unknown node
> originated from. We may get that from the stack trace if that's
> available. But if we could annotate the error with error_context that
> will be super helpful.
Is it really that interesting? If function X lacks coverage for
node type Y, then X is what needs to be fixed. The exact call
chain for any particular failure seems of only marginal interest,
certainly not enough to be building vast new infrastructure for.
regards, tom lane
On Fri, Mar 25, 2022 at 7:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes: > > All these functions are too low level to be helpful to know. Knowing > > the caller might actually give a hint as to where the unknown node > > originated from. We may get that from the stack trace if that's > > available. But if we could annotate the error with error_context that > > will be super helpful. > > Is it really that interesting? If function X lacks coverage for > node type Y, then X is what needs to be fixed. The exact call > chain for any particular failure seems of only marginal interest, > certainly not enough to be building vast new infrastructure for. > I don't think we have tests covering all possible combinations of expression trees. Code coverage reports may not reveal this. I have encountered flaky "unknown expression type" errors. Always ended up changing code to get the stack trace. Having error context helps. -- Best Wishes, Ashutosh Bapat