[
https://issues.apache.org/jira/browse/FLINK-40218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18125827#comment-18125827
]
Sergey Nuyanzin commented on FLINK-40218:
-----------------------------------------
Merged as
[fed133939df542016dbbb0e892a11eacbf9b93ae|https://github.com/apache/flink/commit/fed133939df542016dbbb0e892a11eacbf9b93ae]
> Let callers that already hold parsed JSON build a Variant without a String
> round trip
> -------------------------------------------------------------------------------------
>
> Key: FLINK-40218
> URL: https://issues.apache.org/jira/browse/FLINK-40218
> Project: Flink
> Issue Type: Improvement
> Components: API / Type Serialization System
> Reporter: Ramin Gharib
> Assignee: Ramin Gharib
> Priority: Major
> Labels: pull-request-available
>
> *Description*
> The only way to build a \{{Variant}} from JSON today is from a \{{String}}.
> \{{PARSE_JSON}} and the internal
> \{{BinaryVariantInternalBuilder.parseJson(String)}} both create a Jackson
> parser over the text and walk its tokens in \{{buildJson}}.
> A caller that already holds parsed JSON cannot use that work. It must write
> its data back to a \{{String}}, which flink-core then parses a second time:
> {code}
> bytes --(caller parses)--> tree --(toString)--> String --(flink-core parses
> again)--> Variant
> {code}
> Two things keep callers on this path:
> # *There is no entry point that takes a parser.* \{{parseJson(JsonParser,
> boolean)}} exists but is private.
> # *Jackson types do not cross shading boundaries.* The builder uses Flink's
> shaded \{{org.apache.flink.shaded.jackson2...JsonParser}}. A caller with its
> own, unshaded Jackson cannot pass in its parser or tree. A \{{String}} is the
> only type both sides share.
> *Proposed change*
> Two entry points on the \{{@Internal}} \{{BinaryVariantInternalBuilder}}:
> ||The caller holds||Entry point||
> |Flink's shaded Jackson \{{JsonParser}}|\{{parseJson(JsonParser, boolean)}},
> now public|
> |JSON in another form, such as a tree from its own Jackson|walk it with the
> existing \{{append*}}, \{{addKey}} and \{{finishWritingObject}} methods, and
> call the new \{{appendJsonNumber(String)}} for numbers|
> {\{parseJson(JsonParser, boolean)}} expects the parser on the value's first
> token and leaves it on the value's last token, so a format can read a VARIANT
> field in the middle of a record.
> {\{appendJsonNumber(String)}} reads the literal with the same Jackson factory
> and number code as \{{PARSE_JSON}}. A number is therefore stored byte for
> byte like \{{PARSE_JSON}} stores it: the smallest integer, else a decimal,
> else a finite double. Anything that is not exactly one JSON number fails.
> A walker over an unshaded Jackson tree then looks like this:
> {code:java}
> case NUMBER:
> if (node.isIntegralNumber() && node.canConvertToLong()) {
> builder.appendNumeric(node.longValue());
> } else {
> builder.appendJsonNumber(node.asText());
> }
> {code}
> *Design notes*
> * *Push instead of a token-source interface.* An earlier proposal added a
> pull-based \{{VariantJsonSource}} interface that the builder reads from. It
> is replaced by one push method. The other VARIANT producers push into the
> builder too, such as \{{ToVariantConverter}} for \{{CAST(... AS VARIANT)}}.
> One method is a smaller surface than an interface with an enum.
> \{{PARSE_JSON}} keeps its own code path unchanged.
> * *A String literal, not a \{{Number}}.* \{{PARSE_JSON}} picks the type from
> how a number is written, for example an exponent means DOUBLE. A
> \{{BigDecimal}} cannot tell \{{1e-7}} from \{{0.0000001}}, which
> \{{PARSE_JSON}} stores as DOUBLE and DECIMAL. The text keeps that information.
> * *No hand-written number grammar.* \{{appendJsonNumber}} lets Jackson
> validate the literal, so it rejects \{{+5}}, \{{0x1p4}}, \{{1.5f}} and
> \{{1,5}} exactly like \{{PARSE_JSON}} doe
> *Scope*
> Moving the \{{json}} format onto the new parser entry-XXXXX. That change is
> not behavior-neutral: theformat rounds VARIANT floats through a \{{double}}
> today, so for example \{{1e400}} becomes the string \{{"Infinity"}}. It needs
> its
> own release note.
> *Compatibility*
> Additive and \{{@Internal}}. \{{PARSE_JSON}}, \{{TRY_PA} format do not
> change. \{{addKey}} now looks a key up once instead of twice.
> *Verifying this change*
> * \{{appendJsonNumber}} is byte-for-byte equal to {{tegers at every width,
> values beyond a long,decimals, exponents and surrounding whitespace.
> * It rejects non-JSON literals, other JSON values s trailing content such as
> \{{5 6}}, and numbersoutside the double range.
> * A parser read in the middle of a document is left.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
