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
Attachments:
unknown-node-error-identifiers.patchtext/x-patch; charset=UTF-8; name=unknown-node-error-identifiers.patchDownload
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 6f6a1f4ffc..675789d104 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -270,7 +270,7 @@ exprType(const Node *expr)
type = exprType(((const JsonCoercion *) expr)->expr);
break;
default:
- elog(ERROR, "unrecognized node type: %d", (int) nodeTag(expr));
+ elog(ERROR, "exprType: unrecognized node type: %d", (int) nodeTag(expr));
type = InvalidOid; /* keep compiler quiet */
break;
}
@@ -1017,7 +1017,7 @@ exprCollation(const Node *expr)
}
break;
default:
- elog(ERROR, "unrecognized node type: %d", (int) nodeTag(expr));
+ elog(ERROR, "exprCollation: unrecognized node type: %d", (int) nodeTag(expr));
coll = InvalidOid; /* keep compiler quiet */
break;
}
@@ -1261,7 +1261,7 @@ exprSetCollation(Node *expr, Oid collation)
}
break;
default:
- elog(ERROR, "unrecognized node type: %d", (int) nodeTag(expr));
+ elog(ERROR, "exprSetCollation: unrecognized node type: %d", (int) nodeTag(expr));
break;
}
}
@@ -2531,7 +2531,7 @@ expression_tree_walker(Node *node,
}
break;
default:
- elog(ERROR, "unrecognized node type: %d",
+ elog(ERROR, "expression_tree_walker: unrecognized node type: %d",
(int) nodeTag(node));
break;
}
@@ -3588,7 +3588,7 @@ expression_tree_mutator(Node *node,
}
break;
default:
- elog(ERROR, "unrecognized node type: %d",
+ elog(ERROR, "expression_tree_mutator: unrecognized node type: %d",
(int) nodeTag(node));
break;
}
@@ -4448,7 +4448,7 @@ raw_expression_tree_walker(Node *node,
}
break;
default:
- elog(ERROR, "unrecognized node type: %d",
+ elog(ERROR, "raw_expression_tree_walker: unrecognized node type: %d",
(int) nodeTag(node));
break;
}
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