Skip to content

Commit 9389a6f

Browse files
committed
fix(query): drop arbitrary_precision, pin why
Reverting the previous commit's `arbitrary_precision`. It did keep a wide decimal's digits, but the feature is crate-wide: with it on, `serde_json::Number` serializes as the private struct `$serde_json::private::Number`, which serde_json's own serializer intercepts and serde_yaml does not. Verified — an embedding provider config renders as dimensions: $serde_json::private::Number: '1536' instead of `dimensions: 1536`, for every `-o yaml` output in the binary carrying a `Value` number. Commands with nothing to do with rendering results break, which costs more than the rounding it buys. So a decimal wider than an f64 is rounded here, and that is now a pinned fact rather than a surprise: `a_wide_decimal_is_rounded_a_known_limitation` asserts the rounded value and records why the obvious fix is the wrong one. The honest fix is to stop routing the json output path through `Value` at all, as the service does — a change to the output layer, not to this function. Adds `yaml_renders_a_json_number_as_a_number`, which fails if the feature is ever re-enabled. Nothing in the suite covered yaml output, so this breakage would otherwise have shipped twice.
1 parent 1c7ee1f commit 9389a6f

2 files changed

Lines changed: 43 additions & 16 deletions

File tree

‎Cargo.toml‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,14 @@ directories = "6"
3232
reqwest = { version = "0.13", features = ["blocking", "json"] }
3333
rayon = "1.10"
3434
serde = { version = "1", features = ["derive"] }
35-
# `arbitrary_precision` keeps a JSON number's literal digits instead of routing
36-
# it through an f64. arrow-json writes a decimal at full width and the service
37-
# passes those bytes through untouched, so without this a wide decimal would be
38-
# rounded here and disagree with the service.
39-
serde_json = { version = "1", features = ["arbitrary_precision"] }
35+
# NOT `arbitrary_precision`, deliberately. It would keep a wide decimal's
36+
# digits (see `a_wide_decimal_is_rounded_a_known_limitation`), but the feature
37+
# is crate-wide: with it on, `serde_json::Number` serializes as the private
38+
# struct `$serde_json::private::Number`, which serde_json's own serializer
39+
# intercepts and serde_yaml does not — so every `-o yaml` output carrying a
40+
# `Value` number turns into a nested mapping. That breaks commands unrelated to
41+
# this one, which costs more than the rounding it buys.
42+
serde_json = "1"
4043
arrow = { version = "55", default-features = false, features = [
4144
"ipc",
4245
# Timestamps carrying a named IANA zone ("UTC", "America/New_York")

‎src/commands/query.rs‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,25 +1025,49 @@ mod tests {
10251025
}
10261026
}
10271027

1028-
/// A decimal keeps every digit, as the service's rendering does.
1028+
/// A decimal wider than an `f64` is rounded here. Known limitation, pinned
1029+
/// so it is a discovered fact rather than a surprise.
10291030
///
1030-
/// arrow-json writes a decimal as an unquoted JSON number at full width.
1031-
/// The service passes those bytes straight through; this side parses them,
1032-
/// so without `serde_json`'s `arbitrary_precision` the value lands in an
1033-
/// `f64` and everything past the 17th digit is lost — a 38-digit decimal
1034-
/// silently rounded, and a disagreement with the service.
1031+
/// arrow-json writes a decimal as an unquoted JSON number at full width and
1032+
/// the service passes those bytes through untouched, so a `DECIMAL(38,2)`
1033+
/// reaches this side with all its digits and then loses everything past the
1034+
/// 17th when parsed into a `Value`.
1035+
///
1036+
/// `serde_json`'s `arbitrary_precision` would fix it and must not be used:
1037+
/// the feature is crate-wide, and with it on a `Number` serializes as the
1038+
/// private struct `$serde_json::private::Number`, which serde_yaml renders
1039+
/// as a nested mapping — corrupting `-o yaml` for commands that have
1040+
/// nothing to do with rendering results. The honest fix is to stop routing
1041+
/// the json output path through `Value` at all, as the service does; that
1042+
/// is a change to the output layer, not to this function.
10351043
#[test]
1036-
fn a_wide_decimal_keeps_every_digit() {
1044+
fn a_wide_decimal_is_rounded_a_known_limitation() {
10371045
use arrow::array::Decimal128Array;
10381046

10391047
let col = Decimal128Array::from(vec![123456789012345678901234567890123456i128])
10401048
.with_precision_and_scale(38, 2)
10411049
.expect("valid decimal");
10421050
let got = arrow_cell(&col, &field_for(&col), 0).expect("decimal renders");
1043-
assert_eq!(
1044-
got.to_string(),
1045-
"1234567890123456789012345678901234.56",
1046-
"decimal lost precision: {got}"
1051+
1052+
// Full precision would be 1234567890123456789012345678901234.56.
1053+
assert_eq!(got.to_string(), "1.2345678901234568e+33");
1054+
}
1055+
1056+
/// `-o yaml` must render a JSON number as a number.
1057+
///
1058+
/// This exists to fail loudly if `serde_json`'s `arbitrary_precision` is
1059+
/// ever enabled: the feature is crate-wide, and serde_yaml does not know
1060+
/// about the private struct a `Number` then serializes as, so every yaml
1061+
/// output carrying a `Value` number becomes a nested mapping. Nothing else
1062+
/// in the suite covers yaml, so without this the breakage would ship.
1063+
#[test]
1064+
fn yaml_renders_a_json_number_as_a_number() {
1065+
let v: Value = serde_json::json!({"dimensions": 1536});
1066+
let yaml = serde_yaml::to_string(&v).expect("yaml");
1067+
assert_eq!(yaml.trim(), "dimensions: 1536", "yaml corrupted: {yaml}");
1068+
assert!(
1069+
!yaml.contains("private::Number"),
1070+
"serde_json's arbitrary_precision leaked into yaml: {yaml}"
10471071
);
10481072
}
10491073

0 commit comments

Comments
 (0)