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

7 files changed, 113 insertions, 22 deletions

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index f194ff911b0..511467280f2 100644
--- 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 831a44a2da6..1bf38d7b429 100644
--- 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 03331f6d13f..ec3dfce9c32 100644
--- 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 c4156cf2a66..e9d6e9faf29 100644
--- 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 cf0ce0e44cc..1add673968b 100644
--- 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 20354f04e37..e366c6f51b6 100644
--- 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 1a9d21741fa..5016f29c15a 100644
--- 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]}']);