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

diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out
index c78d372b6383868e06d6097074289ccb2b8ed452..610cb6f3266ffb9aee649a1fe2cfe1eeed6ab5f6 100644 (file)
--- 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 2503d47d14f57a4360ee9d940252cb4abc5d9f71..e806a144960485142f8274b184a50e30e16defc0 100644 (file)
--- 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 d8489cbbdc89e5b77a4dadb6ba10f8811fab1f5f..f6d73b8aa65e89af95b6856daf73a03d9530e07f 100644 (file)
--- 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 4779755fc80f64fe1bc8b475cc5540174855e284..f28897fee48a7f93fd8af6a412ba94d8bbe9b75b 100644 (file)
--- 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"