raminqaf opened a new pull request, #28808:
URL: https://github.com/apache/flink/pull/28808

   ## What is the purpose of the change
   
   `PARSE_JSON` accepted JSON numbers outside the double range, such as 
`1e400`, and silently stored them as `±Infinity`. `Variant.toJson()` then 
emitted bare `Infinity` / `-Infinity` tokens, which are invalid JSON and cannot 
be parsed back by `PARSE_JSON`. This closes that gap by rejecting non-finite 
values on both the parse and the serialize side.
   
   ## Brief change log
   
     - `BinaryVariantInternalBuilder.parseFloatingPoint()` now rejects a 
non-finite result from `getDoubleValue()` with a clear parse error. 
`PARSE_JSON('1e400')` fails loudly and `TRY_PARSE_JSON('1e400')` returns 
`NULL`, so a parsed Variant can never hold a non-finite value.
     - `BinaryVariant.toJson()` now throws a `VariantTypeException` for 
non-finite `DOUBLE` and `FLOAT` values instead of appending invalid `Infinity` 
/ `NaN` tokens. This guards the builder API, which is the only remaining way to 
inject a non-finite value.
     - `parseJson` reuses a single static `JsonFactory` instead of allocating 
one per call.
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   
     - Added 
`BinaryVariantInternalBuilderTest#testParseJsonRejectsNonFiniteNumbers`, 
parameterized over `NaN`, `Infinity`, `-Infinity`, `1e400`, and `-1e400`, 
asserting `parseJson` throws (invalid JSON tokens and finite-but-overflowing 
numbers alike).
     - Added `BinaryVariantTest#testToJsonRejectsNonFiniteDouble` and 
`#testToJsonRejectsNonFiniteFloat`, parameterized over the infinities and 
`NaN`, asserting `toJson()` throws instead of emitting invalid JSON.
   
   ## 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** (both changed classes are `@Internal`; the 
user-visible effect is a semantics change to the `PARSE_JSON` / 
`TRY_PARSE_JSON` SQL functions on out-of-range numbers)
     - The serializers: **no** (the Variant binary encoding is unchanged)
     - The runtime per-record code paths (performance sensitive): **yes** (on 
the `PARSE_JSON` / `toJson()` paths; the only added cost is a constant-time 
finite check, and reusing `JsonFactory` removes a per-call allocation)
     - 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**
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes (please specify the tool below)
   
   <!--
   Generated-by: Claude Code (Claude 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]

Reply via email to