raminqaf opened a new pull request, #29026:
URL: https://github.com/apache/flink/pull/29026
## What is the purpose of the change
Building a `Variant` from JSON currently requires a `String`. A caller that
already holds parsed JSON must serialize it back to a `String`, which
`flink-core` then parses again. This adds a redundant serialize plus re-parse,
and it forces callers to bridge Jackson types across shading boundaries.
This change adds a pull-based token-source abstraction so a caller can feed
JSON tokens straight into the Variant builder, with no `String` round trip and
no Jackson types on the boundary. It is an enabling change. The entry point is
kept `@Internal` for now. A public entry point and format integrations are
follow-ups.
## Brief change log
- Add `VariantJsonSource`, a pull-based token interface in
`org.apache.flink.types.variant`. It exposes only JDK types and an internal
`Token` enum. A number is reported as a single `VALUE_NUMBER` token carrying
its raw literal, so number classification stays in the builder and cannot
diverge across implementations.
- Drive `BinaryVariantInternalBuilder` from a `VariantJsonSource` instead
of a Jackson parser.
- Reimplement `parseJson(String)` on top of the new path through a private
Jackson adapter, so `PARSE_JSON` and `TRY_PARSE_JSON` behavior is unchanged.
## Verifying this change
This change added tests and can be verified as follows:
- `VariantJsonSourceTest` builds a Variant from a custom
`VariantJsonSource` and asserts byte-for-byte equality of the value and
metadata against the `String` path, across scalars, every integer width,
decimal, double, big integer, nesting, and a non-ASCII key.
- The same test covers the error branch a real JSON parser never produces:
a truncated token stream.
- `BinaryVariantInternalBuilderTest` is unchanged and still passes. The
`String` path now flows through the new builder, so it validates equivalence
with the previous behavior.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: no. The new `VariantJsonSource` and the builder are
`@Internal`.
- The serializers: no. The Variant binary encoding is unchanged and is
asserted byte-for-byte.
- The runtime per-record code paths (performance sensitive): yes.
`PARSE_JSON` and `TRY_PARSE_JSON` run per record and now flow String -> Jackson
-> adapter -> builder. Behavior is unchanged and the added cost is one virtual
call per token plus a scan of each numeric literal for `.`, `e`, `E`.
- Anything that affects deployment or recovery: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? JavaDocs
---
One thing to decide before you submit: the template's "Was generative AI
tooling used to co-author this PR?" section. This code was authored with AI
assistance, and the ASF Generative Tooling Guidance asks you to disclose that
here. That disclosure is a policy matter and separate from your "no Claude
attribution" rule, so I've left it out of the draft for you to fill in as you
see fit:
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (Opus 4.8)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
