Improve reporting for syntax errors in multi-line JSON data.

Point to the specific line where the error was detected; the
previous code tended to include several preceding lines as well.
Avoid re-scanning the entire input to recompute which line that
was.  Simplify the logic a bit.  Add test cases.

Simon Riggs and Hamid Akhtar, reviewed by Daniel Gustafsson and myself

Discussion: https://postgr.es/m/CANbhV-EPBnXm3MF_TTWBwwqgn1a1Ghmep9VHfqmNBQ8BT0f+_g@mail.gmail.com

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index f194ff911b0bf17fe3b70c8b48bfc6d629601de6..511467280f26243d6d45fa2a69333fbbdcd6dd5d 100644 (file)
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -641,30 +641,19 @@ report_json_context(JsonLexContext *lex)
        const char *context_start;
        const char *context_end;
        const char *line_start;
-       int                     line_number;
        char       *ctxt;
        int                     ctxtlen;
        const char *prefix;
        const char *suffix;
 
        /* Choose boundaries for the part of the input we will display */
-       context_start = lex->input;
+       line_start = lex->line_start;
+       context_start = line_start;
        context_end = lex->token_terminator;
-       line_start = context_start;
-       line_number = 1;
-       for (;;)
+
+       /* Advance until we are close enough to context_end */
+       while (context_end - context_start >= 50 && context_start < context_end)
        {
-               /* Always advance over newlines */
-               if (context_start < context_end && *context_start == '\n')
-               {
-                       context_start++;
-                       line_start = context_start;
-                       line_number++;
-                       continue;
-               }
-               /* Otherwise, done as soon as we are close enough to context_end */
-               if (context_end - context_start < 50)
-                       break;
                /* Advance to next multibyte character */
                if (IS_HIGHBIT_SET(*context_start))
                        context_start += pg_mblen(context_start);
@@ -694,7 +683,7 @@ report_json_context(JsonLexContext *lex)
        suffix = (lex->token_type != JSON_TOKEN_END && context_end - lex->input < lex->input_length && *context_end != '\n' && *context_end != '\r') ? "..." : "";
 
        return errcontext("JSON data, line %d: %s%s%s",
-                                         line_number, prefix, ctxt, suffix);
+                                         lex->line_number, prefix, ctxt, suffix);
 }
 
 

diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index 831a44a2da632f4349463f537e1cd024237208ad..1bf38d7b4295e40355d5efe1fb24ae83f34ee10f 100644 (file)
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -535,10 +535,12 @@ json_lex(JsonLexContext *lex)
        while (len < lex->input_length &&
                   (*s == ' ' || *s == '\t' || *s == '\n' || *s == '\r'))
        {
-               if (*s == '\n')
+               if (*s++ == '\n')
+               {
                        ++lex->line_number;
-               ++s;
-               ++len;
+                       lex->line_start = s;
+               }
+               len++;
        }
        lex->token_start = s;
 

diff --git a/src/include/common/jsonapi.h b/src/include/common/jsonapi.h
index 03331f6d13fadef4ad342b3167a3d247e1a58920..ec3dfce9c32b3fb5caa204bdfd91fe1078394bec 100644 (file)
--- a/src/include/common/jsonapi.h
+++ b/src/include/common/jsonapi.h
@@ -79,8 +79,8 @@ typedef struct JsonLexContext
        char       *prev_token_terminator;
        JsonTokenType token_type;
        int                     lex_level;
-       int                     line_number;
-       char       *line_start;
+       int                     line_number;    /* line number, starting from 1 */
+       char       *line_start;         /* where that line starts within input */
        StringInfo      strval;
 } JsonLexContext;
 

diff --git a/src/test/regress/expected/json.out b/src/test/regress/expected/json.out
index c4156cf2a66f96cb495b347d5fa37818ad02f188..e9d6e9faf290584e6813cd2ceea02afe5b05212a 100644 (file)
--- a/src/test/regress/expected/json.out
+++ b/src/test/regress/expected/json.out
@@ -272,6 +272,41 @@ LINE 1: SELECT '    '::json;
                ^
 DETAIL:  The input string ended unexpectedly.
 CONTEXT:  JSON data, line 1:     
+-- Multi-line JSON input to check ERROR reporting
+SELECT '{
+               "one": 1,
+               "two":"two",
+               "three":
+               true}'::json; -- OK
+             json             
+------------------------------
+ {                           +
+                 "one": 1,   +
+                 "two":"two",+
+                 "three":    +
+                 true}
+(1 row)
+
+SELECT '{
+               "one": 1,
+               "two":,"two",  -- ERROR extraneous comma before field "two"
+               "three":
+               true}'::json;
+ERROR:  invalid input syntax for type json
+LINE 1: SELECT '{
+               ^
+DETAIL:  Expected JSON value, but found ",".
+CONTEXT:  JSON data, line 3:           "two":,...
+SELECT '{
+               "one": 1,
+               "two":"two",
+               "averyveryveryveryveryveryveryveryveryverylongfieldname":}'::json;
+ERROR:  invalid input syntax for type json
+LINE 1: SELECT '{
+               ^
+DETAIL:  Expected JSON value, but found "}".
+CONTEXT:  JSON data, line 4: ...yveryveryveryveryveryveryveryverylongfieldname":}
+-- ERROR missing value for last field
 --constructors
 -- array_to_json
 SELECT array_to_json(array(select 1 as a));

diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index cf0ce0e44cce05072ae53d0ee4a1869c88fe1aeb..1add673968bfdad6b05c83746371c815e53197f2 100644 (file)
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -272,6 +272,37 @@ LINE 1: SELECT '    '::jsonb;
                ^
 DETAIL:  The input string ended unexpectedly.
 CONTEXT:  JSON data, line 1:     
+-- Multi-line JSON input to check ERROR reporting
+SELECT '{
+               "one": 1,
+               "two":"two",
+               "three":
+               true}'::jsonb; -- OK
+                  jsonb                  
+-----------------------------------------
+ {"one": 1, "two": "two", "three": true}
+(1 row)
+
+SELECT '{
+               "one": 1,
+               "two":,"two",  -- ERROR extraneous comma before field "two"
+               "three":
+               true}'::jsonb;
+ERROR:  invalid input syntax for type json
+LINE 1: SELECT '{
+               ^
+DETAIL:  Expected JSON value, but found ",".
+CONTEXT:  JSON data, line 3:           "two":,...
+SELECT '{
+               "one": 1,
+               "two":"two",
+               "averyveryveryveryveryveryveryveryveryverylongfieldname":}'::jsonb;
+ERROR:  invalid input syntax for type json
+LINE 1: SELECT '{
+               ^
+DETAIL:  Expected JSON value, but found "}".
+CONTEXT:  JSON data, line 4: ...yveryveryveryveryveryveryveryverylongfieldname":}
+-- ERROR missing value for last field
 -- make sure jsonb is passed through json generators without being escaped
 SELECT array_to_json(ARRAY [jsonb '{"a":1}', jsonb '{"b":[2,3]}']);
       array_to_json       

diff --git a/src/test/regress/sql/json.sql b/src/test/regress/sql/json.sql
index 20354f04e3745d80be8b1317797df191b55c5528..e366c6f51b65bdf128034090cc1a9192107d8092 100644 (file)
--- a/src/test/regress/sql/json.sql
+++ b/src/test/regress/sql/json.sql
@@ -59,6 +59,23 @@ SELECT 'trues'::json;                        -- ERROR, not a keyword
 SELECT ''::json;                               -- ERROR, no value
 SELECT '    '::json;                   -- ERROR, no value
 
+-- Multi-line JSON input to check ERROR reporting
+SELECT '{
+               "one": 1,
+               "two":"two",
+               "three":
+               true}'::json; -- OK
+SELECT '{
+               "one": 1,
+               "two":,"two",  -- ERROR extraneous comma before field "two"
+               "three":
+               true}'::json;
+SELECT '{
+               "one": 1,
+               "two":"two",
+               "averyveryveryveryveryveryveryveryveryverylongfieldname":}'::json;
+-- ERROR missing value for last field
+
 --constructors
 -- array_to_json
 

diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 1a9d21741fa5e010f2809c0e734ce762366f66c0..5016f29c15ad73c59f1e340a3dddde3cb73be01b 100644 (file)
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -59,6 +59,23 @@ SELECT 'trues'::jsonb;                       -- ERROR, not a keyword
 SELECT ''::jsonb;                              -- ERROR, no value
 SELECT '    '::jsonb;                  -- ERROR, no value
 
+-- Multi-line JSON input to check ERROR reporting
+SELECT '{
+               "one": 1,
+               "two":"two",
+               "three":
+               true}'::jsonb; -- OK
+SELECT '{
+               "one": 1,
+               "two":,"two",  -- ERROR extraneous comma before field "two"
+               "three":
+               true}'::jsonb;
+SELECT '{
+               "one": 1,
+               "two":"two",
+               "averyveryveryveryveryveryveryveryveryverylongfieldname":}'::jsonb;
+-- ERROR missing value for last field
+
 -- make sure jsonb is passed through json generators without being escaped
 SELECT array_to_json(ARRAY [jsonb '{"a":1}', jsonb '{"b":[2,3]}']);