crates/doc: relax sum inspection to allow numeric strings

Improve the error message to relate that `format: number` may be used. Update catalog test to provide coverage.
estuary · May 31, 2024 · 57e1216 · 57e1216
1 parent db4f493
commit 57e1216
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 18 deletions.
diff --git a/crates/doc/src/shape/inspections.rs b/crates/doc/src/shape/inspections.rs
@@ -1,15 +1,15 @@
 /// This module implements various inspections which can be performed over Shapes.
 use super::*;
 use crate::reduce::Strategy;
-use json::{LocatedProperty, Location};
+use json::{schema::formats::Format, LocatedProperty, Location};
 
 #[derive(thiserror::Error, Debug, Eq, PartialEq)]
 pub enum Error {
     #[error("'{0}' must exist, but is constrained to always be invalid")]
     ImpossibleMustExist(String),
     #[error("'{0}' has reduction strategy, but its parent does not")]
     ChildWithoutParentReduction(String),
-    #[error("{0} has 'sum' reduction strategy, restricted to numbers, but has types {1:?}")]
+    #[error("{0} has 'sum' reduction strategy (restricted to integers, numbers and strings with `format: integer` or `format: number`) but has types {1:?}")]
     SumNotNumber(String, types::Set),
     #[error(
         "{0} has 'merge' reduction strategy, restricted to objects & arrays, but has types {1:?}"
@@ -80,13 +80,14 @@ impl Shape {
             }
         };
 
-        if matches!(self.reduction, Reduction::Strategy(Strategy::Sum))
-            && self.type_ - types::INT_OR_FRAC != types::INVALID
-        {
-            out.push(Error::SumNotNumber(
-                loc.pointer_str().to_string(),
-                self.type_,
-            ));
+        if matches!(self.reduction, Reduction::Strategy(Strategy::Sum)) {
+            match (self.type_ - types::INT_OR_FRAC, &self.string.format) {
+                (types::INVALID, _) => (), // Okay (native numeric only).
+                (types::STRING, Some(Format::Number) | Some(Format::Integer)) => (), // Okay (string-formatted numeric).
+                (type_, _) => {
+                    out.push(Error::SumNotNumber(loc.pointer_str().to_string(), type_));
+                }
+            }
         }
         if matches!(self.reduction, Reduction::Strategy(Strategy::Merge(_)))
             && self.type_ - (types::OBJECT | types::ARRAY) != types::INVALID
@@ -145,6 +146,11 @@ mod test {
         type: object
         reduce: {strategy: merge}
         properties:
+            sum-right-type:
+                reduce: {strategy: sum}
+                type: [number, string]
+                format: integer
+
             sum-wrong-type:
                 reduce: {strategy: sum}
                 type: [number, string]
@@ -207,10 +213,7 @@ mod test {
                 Error::SetInvalidProperty("/-/whoops2".to_owned()),
                 Error::ImpossibleMustExist("/must-exist-but-cannot".to_owned()),
                 Error::ImpossibleMustExist("/nested-array/1".to_owned()),
-                Error::SumNotNumber(
-                    "/sum-wrong-type".to_owned(),
-                    types::INT_OR_FRAC | types::STRING
-                ),
+                Error::SumNotNumber("/sum-wrong-type".to_owned(), types::STRING),
                 Error::MergeNotObjectOrArray("/merge-wrong-type".to_owned(), types::BOOLEAN),
                 Error::ChildWithoutParentReduction("/*/nested-sum".to_owned()),
             ]

diff --git a/crates/validation/tests/snapshots/scenario_tests__shape_inspections.snap b/crates/validation/tests/snapshots/scenario_tests__shape_inspections.snap
@@ -17,7 +17,7 @@ expression: errors
     },
     Error {
         scope: test://example/int-halve#/collections/testing~1int-halve/schema,
-        error: /str has 'sum' reduction strategy, restricted to numbers, but has types "string",
+        error: /str has 'sum' reduction strategy (restricted to integers, numbers and strings with `format: integer` or `format: number`) but has types "string",
     },
     Error {
         scope: test://example/int-halve#/collections/testing~1int-halve/key/0,
@@ -37,7 +37,7 @@ expression: errors
     },
     Error {
         scope: test://example/int-string#/collections/testing~1int-string-rw/readSchema,
-        error: /str has 'sum' reduction strategy, restricted to numbers, but has types "string",
+        error: /str has 'sum' reduction strategy (restricted to integers, numbers and strings with `format: integer` or `format: number`) but has types "string",
     },
     Error {
         scope: test://example/int-string#/collections/testing~1int-string-rw/key/0,

diff --git a/examples/reduction-types/sum.flow.yaml b/examples/reduction-types/sum.flow.yaml
@@ -6,9 +6,10 @@ collections:
       properties:
         key: { type: string }
         value:
-          # Sum only works with types "number" or "integer".
-          # Others will error at build time.
-          type: number
+          # Sum only works with numbers.
+          # Other types will error at build time.
+          type: [number, string]
+          format: number
           reduce: { strategy: sum }
       required: [key]
     key: [/key]
@@ -24,3 +25,12 @@ tests:
         collection: example/reductions/sum
         documents:
           - { key: "key", value: 3.8 }
+    - ingest:
+        collection: example/reductions/sum
+        documents:
+          - { key: "key", value: "2000000000000000000000" }
+          - { key: "key", value: "1000000000010000000000.00005" }
+    - verify:
+        collection: example/reductions/sum
+        documents:
+          - { key: "key", value: "3000000000010000000003.800050000000000" }