mbutrovich commented on PR #4927:
URL:
https://github.com/apache/datafusion-comet/pull/4927#issuecomment-4985862453
The Rust tests cover Bool/Int32/Utf8, nested struct, and null handling with
`ignore_null_fields=true`. They do not cover the branches this PR restructures:
`MaybeQuoted` floats hitting `NaN`/`Infinity` (the reason `MaybeQuoted`
exists), `Raw` decimals and timestamps, escaping inside a `Quoted` value,
unicode, an empty struct, or `ignore_null_fields=false`. The behavior is
preserved, but the safety net for this specific refactor is thin.
- Evidence: to_json.rs `mod test` has `test_primitives` and
`test_nested_struct`, both calling `struct_to_json(..., true)`; no float/NaN,
decimal, timestamp, escaping, unicode, empty-struct, or
`ignore_null_fields=false` case.
- Suggested change: add unit tests for (a) a `Float64` column with rows
`1.5`, `NaN`, `Infinity`, `-Infinity` asserting the special values are quoted
and the finite value is raw, (b) a `Utf8` value containing `"` and `\n`
asserting escaping, (c) `ignore_null_fields=false` asserting null fields
serialize as `"name":null`, and (d) an empty struct asserting `{}`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
