pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
Improved error reporting in format()
3+ messages / 2 participants
[nested] [flat]

* Improved error reporting in format()
@ 2016-01-02 23:57 Jim Nasby <Jim.Nasby@BlueTreble.com>
  2016-01-03 01:19 ` Re: Improved error reporting in format() Jim Nasby <Jim.Nasby@BlueTreble.com>
  0 siblings, 1 reply; 3+ messages in thread

From: Jim Nasby @ 2016-01-02 23:57 UTC (permalink / raw)
  To: pgsql-hackers

The current error message for an invalid format conversion type is 
extremely confusing except in the simplest of uses:

select format( '% moo');
ERROR:  unrecognized conversion type specifier " "

Obviously in that example you can figure out what's going on, but 
frequently format() is used in a complex context where it's not at all 
obvious that format is the problem. Even worse, "conversion" makes it 
sound like a localization issue.

Attached patch clarifies that %-related error messages with hints as 
well as (IMHO) improving the clarity of the message:

select format( '% moo');
ERROR:  unrecognized format() type specifier " "
HINT:  For a single "%" use "%%"

I also made the use of "format()" consistent in all the other error 
messages.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index a89f586..a4552ad 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -4647,7 +4647,8 @@ text_reverse(PG_FUNCTION_ARGS)
 		if (++(ptr) >= (end_ptr)) \
 			ereport(ERROR, \
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE), \
-					 errmsg("unterminated format specifier"))); \
+					 errmsg("unterminated format() type specifier"), \
+					 errhint("For a single \"%%\" use \"%%%%\"."))); \
 	} while (0)
 
 /*
@@ -4779,8 +4780,9 @@ text_format(PG_FUNCTION_ARGS)
 		if (strchr("sIL", *cp) == NULL)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("unrecognized conversion type specifier \"%c\"",
-							*cp)));
+					 errmsg("unrecognized format() type specifier \"%c\"",
+							*cp),
+					 errhint("For a single \"%%\" use \"%%%%\".")));
 
 		/* If indirect width was specified, get its value */
 		if (widthpos >= 0)
@@ -4791,7 +4793,7 @@ text_format(PG_FUNCTION_ARGS)
 			if (arg >= nargs)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("too few arguments for format")));
+						 errmsg("too few arguments for format()")));
 
 			/* Get the value and type of the selected argument */
 			if (!funcvariadic)
@@ -4899,8 +4901,9 @@ text_format(PG_FUNCTION_ARGS)
 				/* should not get here, because of previous check */
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					  errmsg("unrecognized conversion type specifier \"%c\"",
-							 *cp)));
+					  errmsg("unrecognized format() type specifier \"%c\"",
+							 *cp),
+					  errhint("For a single \"%%\" use \"%%%%\".")));
 				break;
 		}
 	}


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

Attachments:

  [text/plain] patch.diff (1.7K, ../../568863FC.9030209@BlueTreble.com/2-patch.diff)
  download | inline diff:
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index a89f586..a4552ad 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -4647,7 +4647,8 @@ text_reverse(PG_FUNCTION_ARGS)
 		if (++(ptr) >= (end_ptr)) \
 			ereport(ERROR, \
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE), \
-					 errmsg("unterminated format specifier"))); \
+					 errmsg("unterminated format() type specifier"), \
+					 errhint("For a single \"%%\" use \"%%%%\"."))); \
 	} while (0)
 
 /*
@@ -4779,8 +4780,9 @@ text_format(PG_FUNCTION_ARGS)
 		if (strchr("sIL", *cp) == NULL)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("unrecognized conversion type specifier \"%c\"",
-							*cp)));
+					 errmsg("unrecognized format() type specifier \"%c\"",
+							*cp),
+					 errhint("For a single \"%%\" use \"%%%%\".")));
 
 		/* If indirect width was specified, get its value */
 		if (widthpos >= 0)
@@ -4791,7 +4793,7 @@ text_format(PG_FUNCTION_ARGS)
 			if (arg >= nargs)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("too few arguments for format")));
+						 errmsg("too few arguments for format()")));
 
 			/* Get the value and type of the selected argument */
 			if (!funcvariadic)
@@ -4899,8 +4901,9 @@ text_format(PG_FUNCTION_ARGS)
 				/* should not get here, because of previous check */
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					  errmsg("unrecognized conversion type specifier \"%c\"",
-							 *cp)));
+					  errmsg("unrecognized format() type specifier \"%c\"",
+							 *cp),
+					  errhint("For a single \"%%\" use \"%%%%\".")));
 				break;
 		}
 	}


^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Improved error reporting in format()
  2016-01-02 23:57 Improved error reporting in format() Jim Nasby <Jim.Nasby@BlueTreble.com>
@ 2016-01-03 01:19 ` Jim Nasby <Jim.Nasby@BlueTreble.com>
  2016-02-11 15:11   ` Re: Improved error reporting in format() Teodor Sigaev <teodor@sigaev.ru>
  0 siblings, 1 reply; 3+ messages in thread

From: Jim Nasby @ 2016-01-03 01:19 UTC (permalink / raw)
  To: pgsql-hackers

On 1/2/16 5:57 PM, Jim Nasby wrote:
> Attached patch clarifies that %-related error messages with hints as
> well as (IMHO) improving the clarity of the message:

Sorry, forgot to update regression tests. New patch attached.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 8683188..9583ed0 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -4647,7 +4647,8 @@ text_reverse(PG_FUNCTION_ARGS)
 		if (++(ptr) >= (end_ptr)) \
 			ereport(ERROR, \
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE), \
-					 errmsg("unterminated format specifier"))); \
+					 errmsg("unterminated format() type specifier"), \
+					 errhint("For a single \"%%\" use \"%%%%\"."))); \
 	} while (0)
 
 /*
@@ -4779,8 +4780,9 @@ text_format(PG_FUNCTION_ARGS)
 		if (strchr("sIL", *cp) == NULL)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("unrecognized conversion type specifier \"%c\"",
-							*cp)));
+					 errmsg("unrecognized format() type specifier \"%c\"",
+							*cp),
+					 errhint("For a single \"%%\" use \"%%%%\".")));
 
 		/* If indirect width was specified, get its value */
 		if (widthpos >= 0)
@@ -4791,7 +4793,7 @@ text_format(PG_FUNCTION_ARGS)
 			if (arg >= nargs)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("too few arguments for format")));
+						 errmsg("too few arguments for format()")));
 
 			/* Get the value and type of the selected argument */
 			if (!funcvariadic)
@@ -4899,8 +4901,9 @@ text_format(PG_FUNCTION_ARGS)
 				/* should not get here, because of previous check */
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					  errmsg("unrecognized conversion type specifier \"%c\"",
-							 *cp)));
+					  errmsg("unrecognized format() type specifier \"%c\"",
+							 *cp),
+					  errhint("For a single \"%%\" use \"%%%%\".")));
 				break;
 		}
 	}
diff --git a/src/test/regress/expected/text.out b/src/test/regress/expected/text.out
index 1587046..a74b9ce 100644
--- a/src/test/regress/expected/text.out
+++ b/src/test/regress/expected/text.out
@@ -211,7 +211,8 @@ ERROR:  too few arguments for format
 select format('Hello %s');
 ERROR:  too few arguments for format
 select format('Hello %x', 20);
-ERROR:  unrecognized conversion type specifier "x"
+ERROR:  unrecognized format() type specifier "x"
+HINT:  For a single "%" use "%%".
 -- check literal and sql identifiers
 select format('INSERT INTO %I VALUES(%L,%L)', 'mytab', 10, 'Hello');
                  format                 
@@ -263,9 +264,11 @@ ERROR:  format specifies argument 0, but arguments are numbered from 1
 select format('%*0$s', 'Hello');
 ERROR:  format specifies argument 0, but arguments are numbered from 1
 select format('%1$', 1);
-ERROR:  unterminated format specifier
+ERROR:  unterminated format() type specifier
+HINT:  For a single "%" use "%%".
 select format('%1$1', 1);
-ERROR:  unterminated format specifier
+ERROR:  unterminated format() type specifier
+HINT:  For a single "%" use "%%".
 -- check mix of positional and ordered placeholders
 select format('Hello %s %1$s %s', 'World', 'Hello again');
             format             


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

Attachments:

  [text/plain] patch.diff (2.9K, ../../5688772A.3020309@BlueTreble.com/2-patch.diff)
  download | inline diff:
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 8683188..9583ed0 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -4647,7 +4647,8 @@ text_reverse(PG_FUNCTION_ARGS)
 		if (++(ptr) >= (end_ptr)) \
 			ereport(ERROR, \
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE), \
-					 errmsg("unterminated format specifier"))); \
+					 errmsg("unterminated format() type specifier"), \
+					 errhint("For a single \"%%\" use \"%%%%\"."))); \
 	} while (0)
 
 /*
@@ -4779,8 +4780,9 @@ text_format(PG_FUNCTION_ARGS)
 		if (strchr("sIL", *cp) == NULL)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("unrecognized conversion type specifier \"%c\"",
-							*cp)));
+					 errmsg("unrecognized format() type specifier \"%c\"",
+							*cp),
+					 errhint("For a single \"%%\" use \"%%%%\".")));
 
 		/* If indirect width was specified, get its value */
 		if (widthpos >= 0)
@@ -4791,7 +4793,7 @@ text_format(PG_FUNCTION_ARGS)
 			if (arg >= nargs)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("too few arguments for format")));
+						 errmsg("too few arguments for format()")));
 
 			/* Get the value and type of the selected argument */
 			if (!funcvariadic)
@@ -4899,8 +4901,9 @@ text_format(PG_FUNCTION_ARGS)
 				/* should not get here, because of previous check */
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					  errmsg("unrecognized conversion type specifier \"%c\"",
-							 *cp)));
+					  errmsg("unrecognized format() type specifier \"%c\"",
+							 *cp),
+					  errhint("For a single \"%%\" use \"%%%%\".")));
 				break;
 		}
 	}
diff --git a/src/test/regress/expected/text.out b/src/test/regress/expected/text.out
index 1587046..a74b9ce 100644
--- a/src/test/regress/expected/text.out
+++ b/src/test/regress/expected/text.out
@@ -211,7 +211,8 @@ ERROR:  too few arguments for format
 select format('Hello %s');
 ERROR:  too few arguments for format
 select format('Hello %x', 20);
-ERROR:  unrecognized conversion type specifier "x"
+ERROR:  unrecognized format() type specifier "x"
+HINT:  For a single "%" use "%%".
 -- check literal and sql identifiers
 select format('INSERT INTO %I VALUES(%L,%L)', 'mytab', 10, 'Hello');
                  format                 
@@ -263,9 +264,11 @@ ERROR:  format specifies argument 0, but arguments are numbered from 1
 select format('%*0$s', 'Hello');
 ERROR:  format specifies argument 0, but arguments are numbered from 1
 select format('%1$', 1);
-ERROR:  unterminated format specifier
+ERROR:  unterminated format() type specifier
+HINT:  For a single "%" use "%%".
 select format('%1$1', 1);
-ERROR:  unterminated format specifier
+ERROR:  unterminated format() type specifier
+HINT:  For a single "%" use "%%".
 -- check mix of positional and ordered placeholders
 select format('Hello %s %1$s %s', 'World', 'Hello again');
             format             


^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Improved error reporting in format()
  2016-01-02 23:57 Improved error reporting in format() Jim Nasby <Jim.Nasby@BlueTreble.com>
  2016-01-03 01:19 ` Re: Improved error reporting in format() Jim Nasby <Jim.Nasby@BlueTreble.com>
@ 2016-02-11 15:11   ` Teodor Sigaev <teodor@sigaev.ru>
  0 siblings, 0 replies; 3+ messages in thread

From: Teodor Sigaev @ 2016-02-11 15:11 UTC (permalink / raw)
  To: Jim Nasby <Jim.Nasby@BlueTreble.com>; pgsql-hackers

Thank you, committed

Jim Nasby wrote:
> On 1/2/16 5:57 PM, Jim Nasby wrote:
>> Attached patch clarifies that %-related error messages with hints as
>> well as (IMHO) improving the clarity of the message:
>
> Sorry, forgot to update regression tests. New patch attached.
>
>
>

-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/


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



^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2016-02-11 15:11 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2016-01-02 23:57 Improved error reporting in format() Jim Nasby <Jim.Nasby@BlueTreble.com>
2016-01-03 01:19 ` Jim Nasby <Jim.Nasby@BlueTreble.com>
2016-02-11 15:11   ` Teodor Sigaev <teodor@sigaev.ru>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox