Allow writing null valued keys in JSON#5065
Conversation
|
As commented on the ticket, the current behaviour is simply incorrect as it will not survive a round-trip. I therefore don't think there is a need to make the prior logic configurable, which I think should simplify this PR |
If you're referring to apache/arrow-rs-object-store#109 then that is a separate issue which this PR doesn't cover. |
|
Aah I see, in that case is there a way we could make this an option instead of a generic. As it stands this is a breaking change, which I'm not sure about |
|
Can you give an example where this might break existing uses? I thought including a default for the new generic parameter for |
|
I could be mistaken but i thought adding new parameters was always a breaking change. Regardless using generics here will result in additional codegen and I'm doubtful will improve performance. Adding a WriterOptions or similar should be more future proof and more consistent with other implementations |
Yeah that makes 👍 I'll work on switching to such |
|
Removed configuring this null behaviour as a generic trait, and changed to be a config that can be set via a new WriterBuilder (akin to arrow-csv writing behaviour) |
tustvold
left a comment
tustvold
left a comment
There was a problem hiding this comment.
This looks good to me, my only comment is I think it should be keep_null_values or possibly preserve_nulls, as null keys are actually illegal in JSON 😅
Yeah I wasn't sure on the proper name. I was gonna leave it as Maybe something like |
|
Perhaps explicit_nulls ? |
Sounds better, renamed to that 👍 |
tustvold
left a comment
tustvold
left a comment
There was a problem hiding this comment.
Looks good to me, thank you
* Allow writing null valued keys in JSON * Trigger * Refactor keep nulls to be runtime config * Rename option * Rename option
2 participants
Which issue does this PR close?
Closes #3774
Rationale for this change
Allow users to write JSON with null values for keys, where previous (and default) is to skip the keys.
What changes are included in this PR?
In arrow-json writer mod, new builder for
Writerwhich allows configuring how null valued keys (and null arrays) should be handled. Will default to skipping keys with null values for backward compatibility.Are there any user-facing changes?
Yes