raminqaf opened a new pull request, #29310:
URL: https://github.com/apache/flink/pull/29310
## What is the purpose of the change
`Variant#toString()` delegated to `toJson()`, which throws for values JSON
cannot represent: NaN, infinity, and nodes with a type id this version does not
know. Such a VARIANT broke logs, test failure messages, and printed query
results. A printed VARIANT holding a JSON `null` showed an empty cell.
This PR keeps `toJson()` strict, makes `toString()` a lenient debug form,
and prints VARIANT results the way `CAST(v AS STRING)` renders them, without
failing.
| Printed value | Before | After
|
|--------------------------|--------------------------|----------------------|
| `{"a":1,"b":["x",null]}` | `{"a":1,"b":["x",null]}` | `{a=1, b=[x, NULL]}`
|
| `[1, NaN]` | fails | `[1, NaN]`
|
| JSON `null` | empty cell | `NULL`
|
| `[1, <type id 31>]` | fails | `[1, <UNKNOWN>]`
|
## Brief change log
- Add an internal `VariantFormatter` interface and move the JSON walk out
of `BinaryVariant` into `JsonVariantFormatter`. `STRICT` backs `toJson()` with
unchanged output. This first commit is a pure move.
- Add `JsonVariantFormatter.LENIENT` for `toString()`. NaN becomes
`"NaN"`, an unknown type id `"<UNKNOWN>"`, and other undecodable data
`"<INVALID>"`. Only the broken node is replaced.
- Print VARIANT through `SqlStringVariantFormatter`, a display mode of the
`CAST(v AS STRING)` rendering that never fails. Bytes print as `x'..'` and a
JSON `null` as `NULL`.
Notes for reviewers:
- Printed VARIANT output changes from JSON to the CAST form, so strings
are no longer quoted. This matches how MAP and ARRAY print. There is no config
option, since printed output is not an API contract.
- `toJson()`, `JSON_STRING`, the json and raw formats, and `CAST(v AS
STRING)` are unchanged.
- The CAST rendering stays in `VariantCastUtils` behind a `display` flag.
The cast owns those rules, and its error messages name the cast target.
- `<UNKNOWN>` and `<INVALID>` split by cause, the same way the strict
exceptions do: `UNKNOWN_PRIMITIVE_TYPE_IN_VARIANT` and `MALFORMED_VARIANT`.
## Verifying this change
This change added tests and can be verified as follows:
- `JsonVariantFormatterTest` covers strict and lenient output for
non-finite numbers, an unknown type id, a malformed value, and a broken
container that already wrote part of its output. The cases use hand-built bytes.
- `SqlStringVariantFormatterTest` checks that display output matches the
cast. It covers NaN, a top-level JSON `null`, bytes that are not UTF-8, unknown
and malformed nodes, and the session zone.
- `CastRulesTest` printing cases now expect the new form. New cases cover
NaN, a JSON `null`, and bytes that are not UTF-8.
- The first commit changes no test. The existing `BinaryVariantTest`
covers it.
- `CastFunctionITCase`, `JsonFunctionsITCase`, `MapFunctionITCase`, and
`VariantSemanticTest` pass unchanged.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): n
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: no. Only the javadoc of `Variant` changes.
- The serializers: no
- The runtime per-record code paths (performance sensitive): yes.
`toJson()` runs per record in `JSON_STRING` and the json and raw formats.
The walk is moved unchanged and adds one `try` and one `Sd per node.
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable. The printed
VARIANT form and the `toJson()`/`toString()` contract are documented
in `data-types.md` and the JavaDocs.
---
##### Was generative AI tooling used to co-author this PR
- [X] Yes (please specify the tool below)
Generated-by: Opus 5.5
--
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]
