make_decoder accepts borrowed DataType instead of owned

[scovich]

Which issue does this PR close?

• Part of [arrow-json] deserialize Variant fields #8987

Rationale for this change

Today's json decoder helper, make_decoder, takes an owned data type whose components are cloned at every level during the recursive decoder initialization process. This breaks pointer stability of the resulting DataType instances that a custom JSON decoder factory would see, vs. those of the schema it and the reader builder were initialized with.

The lack of pointer stability prevents users from creating "path based" decoder factories, that are able to customize decoder behavior based not only on type, but also on the field's path in the schema. See the PathBasedDecoderFactory in arrow-json/tests/custom_decoder_tests.rs of #9259, for an example.

What changes are included in this PR?

By passing &DataType instead, we change code like this:

let decoder = make_decoder(field.data_type().clone(), ...);
Ok(Self {
    data_type,
    decoder,
      ...
})

to this:

let child_decoder = make_decoder(field.data_type(), ...);
Ok(Self {
    data_type: data_type.clone(),
    decoder,
      ...
})

Result: Every call to make_decoder receives a reference to the actual original data type from the builder's input schema. The final decoder Self is unchanged -- it already received a clone and continues to do so.

NOTE: There is one additional clone of the top-level DataType::Struct we create for normal (!is_field) builders. But that's a cheap arc clone of a Fields member.

Are these changes tested?

Yes, existing unit tests validate the change.

Are there any user-facing changes?

No. All functions and data types involved are private -- the array decoders are marked pub but are defined in a private mod with no public re-export that would make them available outside the crate.

[scovich]

NOTE: This Cow::Borrowed is necessary to preserve pointer stability of field.data_type() that would otherwise have to be cloned.

[alamb]

Thanks @scovich

[alamb]

(this PR needs to have some conflicts resolved)

[scovich]

@alamb should be good now

[scovich]

#9271 merged too quickly... this comment was supposed to remain inside the .map call. So I'm restoring this code back to follow the original upstream approach, instead of opening a separate PR just for that.

[alamb]

thank you

[scovich]

@alamb -- Is it normal for MIRI to take O(hours)? It's been stuck here for a very long time:

  ...
test rank::tests::test_bytes ... ok
test rank::tests::test_get_boolean_rank_index ... ok
test rank::tests::test_nullable_booleans ... ok
test rank::tests::test_primitive ... ok

[alamb]

@alamb -- Is it normal for MIRI to take O(hours)? It's been stuck here for a very long time:

It does take a while. I haven't tracked its overall time recently -- maybe some new tests are overly long