[
https://issues.apache.org/jira/browse/FLINK-41298?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ramin Gharib updated FLINK-41298:
---------------------------------
Description:
{{PARSE_JSON}}, {{TRY_PARSE_JSON}}, the {{raw}} format's VARIANT column and the
JSON functions read the first JSON value of their input and ignore what
follows. This is the intended behavior, but no doc states it and no test pins
it.
{noformat}
SELECT JSON_STRING(PARSE_JSON('{"a":1} xyz')); -- {"a":1}
SELECT JSON_STRING(PARSE_JSON('{"a":1}{"a":2}')); -- {"a":1}
SELECT JSON_STRING(TRY_PARSE_JSON('{"a":1} xyz')); -- {"a":1}
SELECT '{"a":1} xyz' IS JSON; -- TRUE
{noformat}
A {{raw}} message that holds newline-delimited JSON without
{{raw.line-delimiter}} keeps only its first document.
*Decision*
Keep the behavior. Spark's {{parse_json}} does the same, and lenient parsing is
Jackson's default. Rejecting the content would turn input that parses today
into errors in running jobs. FLINK-40225 made the same call for {{IS JSON}},
{{JSON_VALUE}} and the other JSON functions.
*Scope*
# A test in {{BinaryVariantInternalBuilderTest}} that pins the behavior for
{{PARSE_JSON}}, {{TRY_PARSE_JSON}} and the {{raw}} format, which share
{{BinaryVariantInternalBuilder.parseJson}}.
# An {{IS JSON}} case in {{JsonFunctionsITCase}} that pins it for the parser
the JSON functions share.
# A one-line note in the docs of {{PARSE_JSON}}, {{TRY_PARSE_JSON}}, the JSON
functions and the {{raw}} format.
This ticket first proposed rejecting the content after the first value. That
proposal was dropped in favor of the scope above.
was:
{{PARSE_JSON}} reads the first JSON value in its input and stops. It never
checks that the input ends there, so anything after that value is dropped
without an error. Two JSON documents in one string come back as the first one,
and the second is lost.
{noformat}
SELECT JSON_STRING(PARSE_JSON('{"a":1} xyz')); -- {"a":1}, expected an
error
SELECT JSON_STRING(PARSE_JSON('[1]]')); -- [1], expected an error
SELECT JSON_STRING(PARSE_JSON('{"a":1}{"a":2}')); -- {"a":1}, expected an
error
SELECT JSON_STRING(TRY_PARSE_JSON('{"a":1} xyz')); -- {"a":1}, expected NULL
{noformat}
Output of {{BinaryVariantInternalBuilder.parseJson(input, false)}} on the 2.3.0
jars:
{noformat}
input result
{"a":1} xyz {"a":1}
[1]] [1]
{"a":1}{"a":2} {"a":1}
{"a":1}\n{"a":2} {"a":1}
1 2 1
hello JsonParseException: Unrecognized token 'hello'
{noformat}
Two entry points on master reach this code:
||Entry point||Builder method||
|{{PARSE_JSON}}, {{TRY_PARSE_JSON}}|{{parseJson(byte[], boolean)}}, through
{{SqlJsonUtils.parseJsonToVariant}}|
|{{raw}} format, VARIANT column|{{parseJson(String, boolean)}}, in
{{RawFormatDeserializationSchema}}|
Both overloads end the same way:
{code:java}
try (JsonParser parser = JSON_FACTORY.createParser(bytes)) {
parser.nextToken();
return parseJson(parser, allowDuplicateKeys); // returns after one value
}
{code}
The same class already rejects content glued to a number, with "Unexpected
content after the JSON number". It does not check what follows the whole value.
*Proposal*
In {{parseJson(String, boolean)}} and {{parseJson(byte[], boolean)}}, call
{{parser.nextToken()}} once more after the value is built, and throw a
{{JsonParseException}} if it is not null. Then {{PARSE_JSON}} fails,
{{TRY_PARSE_JSON}} returns NULL, and the {{raw}} format reports a
deserialization error for the record. Whitespace after the value stays allowed,
because Jackson skips it.
{{parseJson(JsonParser, boolean)}} stays as it is. Its caller owns the parser
and decides what may follow.
*Relation to FLINK-40225*
FLINK-40225 asked for the same check in {{IS JSON}}, {{JSON_VALUE}} and the
other JSON functions. It was closed as Won't Fix: it breaks running jobs, and
lenient parsing is the default in Jackson and Spark. This case differs in two
ways:
# {{PARSE_JSON}} stores what it parsed. The dropped text is lost data, not a
looser validity check. A {{raw}} record that holds newline-delimited JSON
documents keeps only the first one.
# The {{raw}} format's VARIANT support is new in 2.4.0, which is not released
yet. Making it strict now breaks no running job.
*Open question*
{{PARSE_JSON}} has been out since 2.1.0, so rejecting trailing content turns
input that parses today into an error. Should that part go to the dev@ list
first, as FLINK-40225 suggests, while the {{raw}} format gets the check for
2.4.0?
Issue Type: Improvement (was: Bug)
Summary: Document and test that JSON input is read only up to its first
value (was: PARSE_JSON, TRY_PARSE_JSON and the raw format drop content after
the first JSON value)
> Document and test that JSON input is read only up to its first value
> --------------------------------------------------------------------
>
> Key: FLINK-41298
> URL: https://issues.apache.org/jira/browse/FLINK-41298
> Project: Flink
> Issue Type: Improvement
> Components: API / Core, Table SQL / Runtime
> Affects Versions: 2.1.0, 2.2.0, 2.3.0
> Reporter: Ramin Gharib
> Assignee: Ramin Gharib
> Priority: Major
> Labels: pull-request-available
>
> {{PARSE_JSON}}, {{TRY_PARSE_JSON}}, the {{raw}} format's VARIANT column and
> the JSON functions read the first JSON value of their input and ignore what
> follows. This is the intended behavior, but no doc states it and no test pins
> it.
> {noformat}
> SELECT JSON_STRING(PARSE_JSON('{"a":1} xyz')); -- {"a":1}
> SELECT JSON_STRING(PARSE_JSON('{"a":1}{"a":2}')); -- {"a":1}
> SELECT JSON_STRING(TRY_PARSE_JSON('{"a":1} xyz')); -- {"a":1}
> SELECT '{"a":1} xyz' IS JSON; -- TRUE
> {noformat}
> A {{raw}} message that holds newline-delimited JSON without
> {{raw.line-delimiter}} keeps only its first document.
> *Decision*
> Keep the behavior. Spark's {{parse_json}} does the same, and lenient parsing
> is Jackson's default. Rejecting the content would turn input that parses
> today into errors in running jobs. FLINK-40225 made the same call for {{IS
> JSON}}, {{JSON_VALUE}} and the other JSON functions.
> *Scope*
> # A test in {{BinaryVariantInternalBuilderTest}} that pins the behavior for
> {{PARSE_JSON}}, {{TRY_PARSE_JSON}} and the {{raw}} format, which share
> {{BinaryVariantInternalBuilder.parseJson}}.
> # An {{IS JSON}} case in {{JsonFunctionsITCase}} that pins it for the parser
> the JSON functions share.
> # A one-line note in the docs of {{PARSE_JSON}}, {{TRY_PARSE_JSON}}, the JSON
> functions and the {{raw}} format.
> This ticket first proposed rejecting the content after the first value. That
> proposal was dropped in favor of the scope above.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
