Preserve Element Name in ScalarValue::List

[tustvold]

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

ScalarValue::List lacks the ability to preserve the name of its element which prevents round-tripping it correctly

Describe the solution you'd like

ScalarValue::List should store a Box<Field> instead, much like ScalarValue::Struct

[comphead]

Hi @tustvold is it to be consistent with arrow Datatype::List()

    /// A list of some logical data type with variable length.
    List(Box<Field>),

[tustvold]

Yup precisely, when I took a stab at this previously I had to hack around apache/arrow-rs#1649 which was a bit meh.

Let me know if you want to pick one or both of these issues up, as that would be awesome.

[comphead]

I can try with this one.

[comphead]

@tustvold Protobuf serde is expected to pass the entire field value, not just datatype like it was before?

[comphead]

@tustvold for the test, which was before. This represents roundtrip for the list

            ScalarValue::List(
                Some(vec![
                    ScalarValue::Float32(Some(-213.1)),
                    ScalarValue::Float32(None),
                    ScalarValue::Float32(Some(5.5)),
                    ScalarValue::Float32(Some(2.0)),
                    ScalarValue::Float32(Some(1.0)),
                ]),
                Box::new(DataType::List(new_box_field(
                    "level1",
                    DataType::Float32,
                    true,
                ))),

Do you expect after change it has to be

           ScalarValue::List(
                Some(vec![
                    ScalarValue::Float32(Some(-213.1)),
                    ScalarValue::Float32(None),
                    ScalarValue::Float32(Some(5.5)),
                    ScalarValue::Float32(Some(2.0)),
                    ScalarValue::Float32(Some(1.0)),
                ]),
                new_box_field("level1", DataType::Float32, true),
            ),

or

           ScalarValue::List(
                Some(vec![
                    ScalarValue::Float32(Some(-213.1)),
                    ScalarValue::Float32(None),
                    ScalarValue::Float32(Some(5.5)),
                    ScalarValue::Float32(Some(2.0)),
                    ScalarValue::Float32(Some(1.0)),
                ]),
                new_box_field("item", DataType::List(new_box_field("level1", DataType::Float32, true)), true),
            ),

[tustvold]

The latter, you will need to both specify the name of the lists element, and its nullability

[comphead]

The latter, you will need to both specify the name of the lists element, and its nullability

Thanks @tustvold for the quick reply. I was confused because we have the test like

            ScalarValue::List(None, new_box_field("item", DataType::Boolean, true)),

which also looks like list of primitives but with other datatype provided

[tustvold]

This will be a breaking change, and will require updating said tests.