Add column-name hint to log messages generated by inserts when varchars don't fit

Started by Stepan Rutzover 10 years ago2 messages
#1Stepan Rutz
Stepan.Rutz@gmx.de
1 attachment(s)

Hi everybody again,

(Resending this EMail again because apparently I have just send in HTML format, which wasn't my intention)

on our production servers I have quite some errors due to excessively long varchar-values which application-code tries to insert into tables and which don't fit.
The errors look like

ERROR: value too long for type character varying(4)

This is not helping me much. The patch will turn this too

ERROR: value too long for type character varying(4) (hint: column-name is mycolumn)

if the column that was overflown was mycolumn.

The tables have many columns, the statements are not readable and many columns happen to have the same length. Powers of 2 most often for some odd reason ...
I fired up gdb and saw that the error message is generated during the preprocessing of the query where some kind of the constant-folding/constant-elimination happens on the parse-tree. I went ahead and added a try/catch at some point upwards in the call-stack where at least i have the contact of the T_TargetEntry. That has a field resname which gives me exactly the information i need... The column which was overflown. With that info i can fix the application code much more easily. Relation name was out of reach for me, there is a void* passed transparently to the constant-mutator but that is not checkable at the point. That context contains the original top-level statement node however.
The patch just adds a bit of hinting to the error message and goes on.. That is all but really helpful to me and potentially also others.
Attached Patch has more Infos and comments.
Regards from Germany,
Stepan

Stepan Rutz
Phone: +49 (0) 178 654 9284
Email: stepan.rutz@gmx.de
Earth: Brunnenallee 25a, 50226 Frechen, Germany

Attachments:

columname_hint.patchapplication/octet-streamDownload
>From a22ac0f8c6c57894dcee4834e75de9081cf45cd9 Mon Sep 17 00:00:00 2001
From: Stepan Rutz <stepan.rutz@gmx.de>
Date: Tue, 4 Aug 2015 23:33:23 +0200
Subject: [PATCH] Add column-name hint to log messages generated by inserts
 when values don't fit

This patch helps with more logging information when having to deal with
errors in your logfiles saying "value too long".

For most people this should come from wrong application code that attempts to
enter long strings into varchar-columns where they simply don't fit. On a
production system any indication of what column caused this can be helpful.
Merely getting back the column-length is often not helpful if tables have
multiple columns of the same length.

For testing this do:

  create table test1 (c1 varchar(4), c2 varchar(4), c3 varchar(4), c4 varchar(4), c5 varchar(4), c6 varchar(4), c7 varchar(4), c8 varchar(4));

and then triggering the error with:

  insert into test1(c5, c2, c7) values(NULL,'12345',NULL);

Now the error is

  ERROR:  value too long for type character varying(4) (hint: column-name is c8)

and previously it was:

  ERROR:  value too long for type character varying(4)

In a perfect world this should not happen in the first place and it would be
nice to also log the relation etc, but the statement can be logged anyway.

The patch is probably not where you expect it to be, also the relation name
isn't logged as well. It's in the void* context that is opaquely passed to
the mutator. if i understand the intensions of the code correctly this void*
pointer should just be passed through here. The patch captures the failures
of the static node processing on the varchar function call a bit upwards in
the callstack by adding a try/catch block, adds some info to the err-msg
and then rethrows.

It really helps my production scenarios, where there is wide tables, many
statements, a lot of logging going on and many many colmns and errors that
occur rarely, but are breaking things for the customers.
---
 src/backend/nodes/nodeFuncs.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index c517dfd..5246601 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2632,7 +2632,40 @@ expression_tree_mutator(Node *node,
 				TargetEntry *newnode;
 
 				FLATCOPY(newnode, targetentry, TargetEntry);
-				MUTATE(newnode->expr, targetentry->expr, Expr *);
+				/* Wrap Mutate in a try/catch so that we can potentially log more
+				 * info to the server log. The actual case here is an insert of
+				 * a varchar value that is too long, which fails at this point
+				 * during constant expression processing.
+				 */
+				PG_TRY();
+				{
+					MUTATE(newnode->expr, targetentry->expr, Expr *);
+				}
+				PG_CATCH();
+				{
+					/* Provide more error information here, if one has a long
+					 * insert statement with a values-clause that contains many columns
+					 * then in case the mutation fails we want to know what the target field
+					 * is that caused the problem. This is the case when we have a too long
+					 * string that we try to put into a varchar column whose size is exceeded.
+					 * Without this block we just get an error saying "value too long" but we
+					 * are left without any column information and in our application code we
+					 * would be force to analyze the values of statement beforehand to get any
+					 * indication whatsoever. The target relation doesn't seem to be reachable
+					 * from this place in the code easily unless casting the context pointer.
+					 */
+					ErrorData *edata = CopyErrorData();
+					if (edata != NULL && edata->message != NULL && targetentry->resname != NULL && strlen(targetentry->resname) > 0)
+					{
+						/* Though errmsg() frees edata->message this works fine, since edata->message
+						 * is destroyed at the end we can use it as an argument before. */
+						errmsg("%s (hint: column-name is %s)", edata->message, targetentry->resname);
+					}
+					if (edata != NULL)
+						FreeErrorData(edata);
+					PG_RE_THROW();
+				}
+				PG_END_TRY();
 				return (Node *) newnode;
 			}
 			break;
-- 
1.9.1

#2Robert Haas
robertmhaas@gmail.com
In reply to: Stepan Rutz (#1)
Re: Add column-name hint to log messages generated by inserts when varchars don't fit

On Wed, Aug 5, 2015 at 6:39 AM, Stepan Rutz <Stepan.Rutz@gmx.de> wrote:

on our production servers I have quite some errors due to excessively long varchar-values which application-code tries to insert into tables and which don't fit.
The errors look like

ERROR: value too long for type character varying(4)

This is not helping me much. The patch will turn this too

ERROR: value too long for type character varying(4) (hint: column-name is mycolumn)

if the column that was overflown was mycolumn.

expression_tree_walker is used in enough different places that you
can't really modify that like this. It'll have too many side effects
that may not be good.

Generally, the way to stick some useful information into the error
message is with an error context callback, rather than by appending to
the primary error message.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers