Skip to content

serde rc feature might not be needed? #989

@carols10cents

Description

@carols10cents

This is low priority, and I'm happy to send in a PR for this if it's determined we can remove the rc feature.

I'm analyzing dependencies and features in IOx, which is also leading me to analyze dependencies and features in IOx's dependencies.

I noticed arrow enables serde's rc feature:

serde = { version = "1.0", features = ["rc"] }

serde's docs say:

--features rc

Opt into impls for Rc and Arc. Serializing and deserializing these types does not preserve identity and may result in multiple copies of the same data. Be sure that this is what you want before enabling this feature.

Serializing a data structure containing reference-counted pointers will serialize a copy of the inner value of the pointer each time a pointer is referenced within the data structure. Serialization will not attempt to deduplicate these repeated data.

Deserializing a data structure containing reference-counted pointers will not attempt to deduplicate references to the same data. Every deserialized pointer will end up with a strong count of 1.

The feature was added in this PR made 3 years ago. It adds derive(Serialize, Deserialize) to DataType, Field, and Schema, none of which contain any Rc<T> or Arc<T> as far as I can tell. It only adds a test for a DataType::Struct that contains Fields, but Schema contains Fields too, so I think that coverage is enough.

I tried taking the rc feature out and arrow's tests still pass.

@andygrove I don't suppose you remember why you enabled the rc feature of serde in a PR made 3 years ago? 😂

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions