From e07e2a40bd0c3c02a9baf2e5ddccf665e73208fb Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 31 Mar 2020 10:30:59 -0400 Subject: [PATCH] Improve error messages in ltree_in and lquery_in. Ensure that the type name is mentioned in all cases (bare "syntax error" isn't that helpful). Avoid using the term "level", since that's not used in the documentation. Phrase error position reports as "at character N" not "in position N"; the latter seems ambiguous, and it's certainly not how we say it elsewhere. For the same reason, make the character position values be 1-based not 0-based. Provide a position in more cases. (I continued to leave that out of messages complaining about end-of-input, where it seemed pointless, as well as messages complaining about overall input complexity, where fingering any one part of the input would be arbitrary.) Discussion: https://postgr.es/m/15582.1585529626@sss.pgh.pa.us --- contrib/ltree/expected/ltree.out | 13 ++- contrib/ltree/ltree_io.c | 99 ++++++++++--------- contrib/ltree/sql/ltree.sql | 1 + .../expected/ltree_plpython.out | 2 +- 4 files changed, 63 insertions(+), 52 deletions(-) diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out index c78d372b63..610cb6f326 100644 --- a/contrib/ltree/expected/ltree.out +++ b/contrib/ltree/expected/ltree.out @@ -464,7 +464,7 @@ SELECT nlevel(('1' || repeat('.1', 65534))::ltree); (1 row) SELECT nlevel(('1' || repeat('.1', 65535))::ltree); -ERROR: number of ltree levels (65536) exceeds the maximum allowed (65535) +ERROR: number of ltree labels (65536) exceeds the maximum allowed (65535) SELECT nlevel(('1' || repeat('.1', 65534))::ltree || '1'); ERROR: number of ltree levels (65536) exceeds the maximum allowed (65535) SELECT ('1' || repeat('.1', 65534))::lquery IS NULL; @@ -474,7 +474,7 @@ SELECT ('1' || repeat('.1', 65534))::lquery IS NULL; (1 row) SELECT ('1' || repeat('.1', 65535))::lquery IS NULL; -ERROR: number of lquery levels (65536) exceeds the maximum allowed (65535) +ERROR: number of lquery items (65536) exceeds the maximum allowed (65535) SELECT '*{65535}'::lquery; lquery ---------- @@ -485,7 +485,7 @@ SELECT '*{65536}'::lquery; ERROR: lquery syntax error LINE 1: SELECT '*{65536}'::lquery; ^ -DETAIL: Low limit (65536) exceeds the maximum allowed (65535). +DETAIL: Low limit (65536) exceeds the maximum allowed (65535), at character 3. SELECT '*{,65534}'::lquery; lquery ----------- @@ -502,7 +502,12 @@ SELECT '*{,65536}'::lquery; ERROR: lquery syntax error LINE 1: SELECT '*{,65536}'::lquery; ^ -DETAIL: High limit (65536) exceeds the maximum allowed (65535). +DETAIL: High limit (65536) exceeds the maximum allowed (65535), at character 4. +SELECT '*{4,3}'::lquery; +ERROR: lquery syntax error +LINE 1: SELECT '*{4,3}'::lquery; + ^ +DETAIL: Low limit (4) is greater than high limit (3), at character 5. SELECT '1.2'::ltree < '2.2'::ltree; ?column? ---------- diff --git a/contrib/ltree/ltree_io.c b/contrib/ltree/ltree_io.c index 2503d47d14..e806a14496 100644 --- a/contrib/ltree/ltree_io.c +++ b/contrib/ltree/ltree_io.c @@ -17,12 +17,6 @@ PG_FUNCTION_INFO_V1(lquery_in); PG_FUNCTION_INFO_V1(lquery_out); -#define UNCHAR ereport(ERROR, \ - (errcode(ERRCODE_SYNTAX_ERROR), \ - errmsg("syntax error at position %d", \ - pos))); - - typedef struct { char *start; @@ -47,7 +41,12 @@ ltree_in(PG_FUNCTION_ARGS) ltree *result; ltree_level *curlevel; int charlen; - int pos = 0; + int pos = 1; /* character position for error messages */ + +#define UNCHAR ereport(ERROR, \ + errcode(ERRCODE_SYNTAX_ERROR), \ + errmsg("ltree syntax error at character %d", \ + pos)) ptr = buf; while (*ptr) @@ -61,7 +60,7 @@ ltree_in(PG_FUNCTION_ARGS) if (num + 1 > LTREE_MAX_LEVELS) ereport(ERROR, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("number of ltree levels (%d) exceeds the maximum allowed (%d)", + errmsg("number of ltree labels (%d) exceeds the maximum allowed (%d)", num + 1, LTREE_MAX_LEVELS))); list = lptr = (nodeitem *) palloc(sizeof(nodeitem) * (num + 1)); ptr = buf; @@ -88,10 +87,10 @@ ltree_in(PG_FUNCTION_ARGS) if (lptr->wlen > LTREE_LABEL_MAX_CHARS) ereport(ERROR, (errcode(ERRCODE_NAME_TOO_LONG), - errmsg("name of level is too long"), - errdetail("Name length is %d, must " - "be < 256, in position %d.", - lptr->wlen, pos))); + errmsg("label string is too long"), + errdetail("Label length is %d, must be at most %d, at character %d.", + lptr->wlen, LTREE_LABEL_MAX_CHARS, + pos))); totallen += MAXALIGN(lptr->len + LEVEL_HDRSIZE); lptr++; @@ -115,10 +114,9 @@ ltree_in(PG_FUNCTION_ARGS) if (lptr->wlen > LTREE_LABEL_MAX_CHARS) ereport(ERROR, (errcode(ERRCODE_NAME_TOO_LONG), - errmsg("name of level is too long"), - errdetail("Name length is %d, must " - "be < 256, in position %d.", - lptr->wlen, pos))); + errmsg("label string is too long"), + errdetail("Label length is %d, must be at most %d, at character %d.", + lptr->wlen, LTREE_LABEL_MAX_CHARS, pos))); totallen += MAXALIGN(lptr->len + LEVEL_HDRSIZE); lptr++; @@ -126,8 +124,8 @@ ltree_in(PG_FUNCTION_ARGS) else if (!(state == LTPRS_WAITNAME && lptr == list)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("syntax error"), - errdetail("Unexpected end of line."))); + errmsg("ltree syntax error"), + errdetail("Unexpected end of input."))); result = (ltree *) palloc0(LTREE_HDRSIZE + totallen); SET_VARSIZE(result, LTREE_HDRSIZE + totallen); @@ -144,6 +142,8 @@ ltree_in(PG_FUNCTION_ARGS) pfree(list); PG_RETURN_POINTER(result); + +#undef UNCHAR } Datum @@ -208,7 +208,12 @@ lquery_in(PG_FUNCTION_ARGS) bool hasnot = false; bool wasbad = false; int charlen; - int pos = 0; + int pos = 1; /* character position for error messages */ + +#define UNCHAR ereport(ERROR, \ + errcode(ERRCODE_SYNTAX_ERROR), \ + errmsg("lquery syntax error at character %d", \ + pos)) ptr = buf; while (*ptr) @@ -230,7 +235,7 @@ lquery_in(PG_FUNCTION_ARGS) if (num > LQUERY_MAX_LEVELS) ereport(ERROR, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("number of lquery levels (%d) exceeds the maximum allowed (%d)", + errmsg("number of lquery items (%d) exceeds the maximum allowed (%d)", num, LQUERY_MAX_LEVELS))); curqlevel = tmpql = (lquery_level *) palloc0(ITEMSIZE * num); ptr = buf; @@ -305,10 +310,10 @@ lquery_in(PG_FUNCTION_ARGS) if (lptr->wlen > LTREE_LABEL_MAX_CHARS) ereport(ERROR, (errcode(ERRCODE_NAME_TOO_LONG), - errmsg("name of level is too long"), - errdetail("Name length is %d, must " - "be < 256, in position %d.", - lptr->wlen, pos))); + errmsg("label string is too long"), + errdetail("Label length is %d, must be at most %d, at character %d.", + lptr->wlen, LTREE_LABEL_MAX_CHARS, + pos))); state = LQPRS_WAITVAR; } @@ -321,10 +326,10 @@ lquery_in(PG_FUNCTION_ARGS) if (lptr->wlen > LTREE_LABEL_MAX_CHARS) ereport(ERROR, (errcode(ERRCODE_NAME_TOO_LONG), - errmsg("name of level is too long"), - errdetail("Name length is %d, must " - "be < 256, in position %d.", - lptr->wlen, pos))); + errmsg("label string is too long"), + errdetail("Label length is %d, must be at most %d, at character %d.", + lptr->wlen, LTREE_LABEL_MAX_CHARS, + pos))); state = LQPRS_WAITLEVEL; curqlevel = NEXTLEV(curqlevel); @@ -361,10 +366,10 @@ lquery_in(PG_FUNCTION_ARGS) if (low < 0 || low > LTREE_MAX_LEVELS) ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("lquery syntax error"), - errdetail("Low limit (%d) exceeds the maximum allowed (%d).", - low, LTREE_MAX_LEVELS))); + errdetail("Low limit (%d) exceeds the maximum allowed (%d), at character %d.", + low, LTREE_MAX_LEVELS, pos))); curqlevel->low = (uint16) low; state = LQPRS_WAITND; @@ -379,11 +384,17 @@ lquery_in(PG_FUNCTION_ARGS) int high = atoi(ptr); if (high < 0 || high > LTREE_MAX_LEVELS) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("lquery syntax error"), + errdetail("High limit (%d) exceeds the maximum allowed (%d), at character %d.", + high, LTREE_MAX_LEVELS, pos))); + else if (curqlevel->low > high) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("lquery syntax error"), - errdetail("High limit (%d) exceeds the maximum allowed (%d).", - high, LTREE_MAX_LEVELS))); + errdetail("Low limit (%d) is greater than high limit (%d), at character %d.", + curqlevel->low, high, pos))); curqlevel->high = (uint16) high; state = LQPRS_WAITCLOSE; @@ -441,7 +452,7 @@ lquery_in(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("lquery syntax error"), - errdetail("Unexpected end of line."))); + errdetail("Unexpected end of input."))); lptr->len = ptr - lptr->start - ((lptr->flag & LVAR_SUBLEXEME) ? 1 : 0) - @@ -451,15 +462,14 @@ lquery_in(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("lquery syntax error"), - errdetail("Unexpected end of line."))); + errdetail("Unexpected end of input."))); if (lptr->wlen > LTREE_LABEL_MAX_CHARS) ereport(ERROR, (errcode(ERRCODE_NAME_TOO_LONG), - errmsg("name of level is too long"), - errdetail("Name length is %d, must " - "be < 256, in position %d.", - lptr->wlen, pos))); + errmsg("label string is too long"), + errdetail("Label length is %d, must be at most %d, at character %d.", + lptr->wlen, LTREE_LABEL_MAX_CHARS, pos))); } else if (state == LQPRS_WAITOPEN) curqlevel->high = LTREE_MAX_LEVELS; @@ -467,7 +477,7 @@ lquery_in(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("lquery syntax error"), - errdetail("Unexpected end of line."))); + errdetail("Unexpected end of input."))); curqlevel = tmpql; totallen = LQUERY_HDRSIZE; @@ -483,13 +493,6 @@ lquery_in(PG_FUNCTION_ARGS) lptr++; } } - else if (curqlevel->low > curqlevel->high) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("lquery syntax error"), - errdetail("Low limit (%d) is greater than upper (%d).", - curqlevel->low, curqlevel->high))); - curqlevel = NEXTLEV(curqlevel); } @@ -543,6 +546,8 @@ lquery_in(PG_FUNCTION_ARGS) pfree(tmpql); PG_RETURN_POINTER(result); + +#undef UNCHAR } Datum diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql index d8489cbbdc..f6d73b8aa6 100644 --- a/contrib/ltree/sql/ltree.sql +++ b/contrib/ltree/sql/ltree.sql @@ -100,6 +100,7 @@ SELECT '*{65536}'::lquery; SELECT '*{,65534}'::lquery; SELECT '*{,65535}'::lquery; SELECT '*{,65536}'::lquery; +SELECT '*{4,3}'::lquery; SELECT '1.2'::ltree < '2.2'::ltree; SELECT '1.2'::ltree <= '2.2'::ltree; diff --git a/contrib/ltree_plpython/expected/ltree_plpython.out b/contrib/ltree_plpython/expected/ltree_plpython.out index 4779755fc8..f28897fee4 100644 --- a/contrib/ltree_plpython/expected/ltree_plpython.out +++ b/contrib/ltree_plpython/expected/ltree_plpython.out @@ -38,6 +38,6 @@ $$; -- because it will try to parse the Python list as an ltree input -- string. SELECT test2(); -ERROR: syntax error at position 0 +ERROR: ltree syntax error at character 1 CONTEXT: while creating return value PL/Python function "test2"