Allow writing null valued keys in JSON

[Jefffrey]

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 Writer which 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

[tustvold]

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

[Jefffrey]

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 #5066 then that is a separate issue which this PR doesn't cover.

[tustvold]

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

[Jefffrey]

Can you give an example where this might break existing uses? I thought including a default for the new generic parameter for Writer would make this backwards compatible, but would be happy to be proven wrong.

[tustvold]

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

[Jefffrey]

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

[Jefffrey]

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]

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 😅

[Jefffrey]

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 keep_nulls but I felt that could be confusing since it might imply affecting null values in arrays which should always be kept regardless.

Maybe something like preserve_object_nulls?

[tustvold]

Perhaps explicit_nulls ?

[Jefffrey]

Perhaps explicit_nulls ?

Sounds better, renamed to that 👍

[tustvold]

Looks good to me, thank you