fix(rust)!: stop forcing serde_json preserve_order on downstream crates

[nielspardon]

Fixes the Rust half of substrait-io/substrait-rs#502.

Problem

substrait-extensions and substrait-prost both declared serde_json with the preserve_order feature:

serde_json = { version = "1.0.145", features = ["preserve_order"] }

preserve_order is global. Because of Cargo feature unification, enabling it anywhere in a dependency graph switches serde_json::Map from BTreeMap to IndexMap for every crate in the build, not just ours. Downstream users had no way to opt out short of patching these crates.

The issue names only substrait, which was exactly right when it was filed on 2026-06-15: at that point substrait-rs was a single crate with a single serde_json declaration and its own build.rs. substrait-extensions and substrait-prost did not exist yet — they were created three days later in #20, which copied the preserve_order declaration into both.

So the fix surface grew from one crate to three after the report, and substrait-rs only began depending on the packaged crates in substrait-io/substrait-rs#507. Today all three carry it, and the two here are non-optional dependencies of substrait — so fixing substrait-rs alone would change nothing:

Crate	Gating
substrait	none — unconditional (fixed separately in substrait-rs)
substrait-extensions	none — unconditional
substrait-prost	optional, via its serde feature

Root cause

Exactly one thing needed order preservation: the free-form-object arm of the Type schema (oneOf: [string, object]), which carries a named struct's fields. Their order is semantically significant — see substrait-io/substrait#915.

build.rs already asked typify to preserve order:

.with_map_type("::indexmap::IndexMap")

But with_map_type does not apply to this type, and that is documented, intended typify behaviour — not an oversight on typify's part. typify hardcodes ::serde_json::Map<String, Value> whenever a map's key is String and its value is arbitrary JSON (typify-impl-0.7.0/src/type_entry.rs:1821), which is exactly the shape of $defs/type's object arm. From typify's README:

Note that for a custom map type to work you must have T defined to generate a struct as described in Objects. If T is not defined, typify will generate code using a serde_json::Map<String, serde_json::Value> instead.

The typify maintainer raised this precise line while reviewing the PR that added with_map_type (oxidecomputer/typify#708) — "I think this is the right thing to do in this case, but I do want to point out that this is an exceptional case ... document that the override doesn't apply to this" — so it is a considered design decision.

The practical consequence for us is that with_map_type never covered the one type that mattered, and the ordering guarantee rested on preserve_order instead.

Fix

Rewrite just that one type in the generated output to IndexMap, which preserves insertion order unconditionally, then drop preserve_order from both crates.

The rewrite is anchored on the whole Type block rather than applied globally, so the diff against the previously generated code is exactly 3 lines:

-    Object(::serde_json::Map<::std::string::String, ::serde_json::Value>),
+    Object(::indexmap::IndexMap<::std::string::String, ::serde_json::Value>),
-impl ::std::convert::From<::serde_json::Map<::std::string::String, ::serde_json::Value>>
+impl ::std::convert::From<::indexmap::IndexMap<::std::string::String, ::serde_json::Value>>
-        value: ::serde_json::Map<::std::string::String, ::serde_json::Value>,
+        value: ::indexmap::IndexMap<::std::string::String, ::serde_json::Value>,

metadata and required_options hold order-insensitive data ("arbitrary data created by the dialect author") and keep their current public types. Option<...> count in the generated output is unchanged at 366, identical to before.

substrait-prost never referenced serde_json at all — the pbjson-generated code does not use it — so dropping the feature there is a no-op beyond removing the contagion. (The dependency itself now looks unused; I left it in place to keep this PR focused, but it may be worth removing separately.)

Why post-process codegen, and why that is safe here

The obvious objection to rewriting generated source in a build script is silent failure: a typify or prettyplease upgrade reflows the output, the pattern stops matching, serde_json::Map quietly returns, and nothing fails — leaving the ordering guarantee broken again with no signal.

So the rewrite asserts it matched exactly once. Verified it fires by perturbing the anchor:

thread 'main' panicked at build.rs:158:5:
assertion `left == right` failed: expected exactly one `Type` block to rewrite in
`text/simple_extensions_schema.yaml`; the generated formatting changed and
`rewrite_type_object_map` needs updating

That turns the failure mode from a silent regression into a build break naming the file and the function to fix.

Alternatives considered

Two supported typify APIs also work, and I prototyped both:

• with_conversion on bare {"type": "object"} — works, but also wraps 70 unrelated fields (metadata, required_options) in an extra Option layer, pushing the Option count from 366 to 576. Cause: typify decides optionality from TypeEntryDetails (structs.rs:440-444); a converted type is Native, which falls through to Required and gets wrapped. Passing TypeSpaceImpl::Default does not suppress it — has_default never consults the impls list. Strictly larger breaking change for no benefit.
• with_replacement("Type", ...) — equally surgical, but moves Type out of the generated module, so it needs a hand-written ~20-line enum mirroring the schema plus a re-export to keep text::simple_extensions::Type resolving. Viable fallback if the team's position is that build scripts must not post-process generated source; the cost is a hand-maintained schema mirror.

Since the typify behaviour is intended and documented, there is no upstream bug to wait on — solving this on our side is the correct level. (A feature request to make the fallback configurable would have to argue against an explicit prior decision. There is a reasonable argument available — serde_json::Map's ordering is only controllable through a global feature, which is the whole reason this leaked downstream — but that is a separate conversation and not a blocker here.)

The better thing to wait on is substrait-io/substrait#915, which may remove the need for this entirely; see below.

Verification

Prototyped against the published 0.87.0 crates (this repo's build.rs is byte-identical to the published one) with substrait-rs pointed at both patched crates via [patch.crates-io]:

• preserve_order absent from the whole graph, confirmed with cargo tree -e features --all-features -i serde_json.
• substrait-rs test suite green on --all-features (63), --no-default-features --features parse,extensions (60), and --no-default-features.
• Added an end-to-end check deserializing an extension YAML fragment through serde_yaml into the generated types and asserting field order survives — passes without preserve_order.
• cargo fmt --check and cargo clippy --all-targets clean.

Note for reviewers: the existing order test is currently masked

substrait-rs's test_named_struct_field_order_stability passes under --all-features even with the ordering guarantee fully broken, because substrait-prost's serde feature drags preserve_order back in. I confirmed this by removing preserve_order without the fix — the suite went green anyway. Only on a feature set excluding serde does it correctly fail:

assertion `left == right` failed: field order should be preserved
  left: ["alpha", "beta"]
 right: ["beta", "alpha"]

CI needs a job on a feature set that excludes serde, or this guarantee stays untested. That belongs in the companion substrait-rs PR.

Breaking change

text::simple_extensions::Type::Object now holds indexmap::IndexMap<String, serde_json::Value> instead of serde_json::Map<String, serde_json::Value>. Mechanical for callers — in substrait-rs it was a 4-line change, and its parse path already collected into an IndexMap.

Worth flagging: this crate's version tracks Substrait spec releases, so a breaking Rust API change cannot be expressed as a major bump. Please sanity-check the release timing.

This is an interim fix by design

Worth recording for whoever reads this later: preserving YAML key order here is a workaround for an open spec question, not settled behaviour.

Struct fields in Substrait are referenced by zero-indexed ordinal position, not by name (fields are unnamed and references are always based on zero-index ordinal positions), and a YAML named struct is sugar whose key order defines those ordinals. But JSON objects are formally unordered (RFC 8259), so the schema currently relies on something JSON does not guarantee. That is substrait-io/substrait#915, still open, which proposes four resolutions — key/value pair lists, parallel names/types arrays, lexicographic ordering, or dropping object notation. Under the last two, order preservation becomes unnecessary or the object arm disappears.

#915 was filed while reviewing substrait-io/substrait-rs#395 — the PR that introduced preserve_order — and that review thread is explicit that the map-type approach was meant to be temporary: "it sounds like the right decision will be to leverage an alternative map type using something like an IndexMap. This would just be until the upstream is solved."

So this PR keeps that interim contract intact while removing the global feature that leaked out of it. It is deliberately small and localised so it can be deleted cleanly when #915 lands.

Follow-ups

• Companion substrait-rs PR dropping its own preserve_order and adapting the 4 lines (not yet opened — this PR should land first).
• Revisit once substrait-io/substrait#915 is resolved (see below) — under two of its four candidate resolutions this rewrite can simply be deleted.

🤖 Generated with AI