Skip to content

Conversation

AdamGS
Copy link
Contributor

@AdamGS AdamGS commented Oct 6, 2025

Which issue does this PR close?

With this change, its possible to compile the core crate without pulling serde_derive, which is will be only required for arrow-avro and arrow-schema/serde.

Rationale for this change

Improve compile time and reduce number of dependencies and binary size in some cases.

What changes are included in this PR?

  1. Use serde_core when possible
  2. Manually implement Serialize/Deserialize for canonical extension type metadata.

Are these changes tested?

Covered by existing tests

Are there any user-facing changes?

No

@github-actions github-actions bot added the arrow Changes to the arrow crate label Oct 6, 2025
@AdamGS AdamGS force-pushed the adamg/serde-core-8451 branch from 785f56e to 324dcfa Compare October 6, 2025 10:52
where
D: Deserializer<'de>,
{
#[derive(Debug)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my guess is that this is auto-generated code. I personally would prefer if we wouldn't nest structs/enums/impls/etc. into methods. It makes the code rather hard to read (for auto-generated code there are usually good reasons why this is done this way, e.g. due to shadowing).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely autogenerated-inspired, I'll pull all the inner definition outside.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@AdamGS AdamGS force-pushed the adamg/serde-core-8451 branch from 324dcfa to 243f295 Compare October 6, 2025 10:53
@crepererum
Copy link
Contributor

This currently is only semi-effective until BurntSushi/rust-csv#403 is merged and released. But if we try this upstream change by adding the following to Cargo.toml:

[patch.crates-io]
csv = {git = "https://github.com/paolobarbolini/rust-csv.git", branch = "switch-to-serde-core"}

and then we get all crates for a full-featured arrow (but without dev dependencies because criterion uses derive-based serialization):

$ cargo tree -p arrow --all-features -e=no-dev

then there's no serde_derive left and no syn and no proc-macro2. So I would call this a win.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change serde dependency to serde_core where applicable
2 participants