diff --git a/crates/analysis/src/attribute_join.rs b/crates/analysis/src/attribute_join.rs index 7b75c7e0b..fa9505fb8 100644 --- a/crates/analysis/src/attribute_join.rs +++ b/crates/analysis/src/attribute_join.rs @@ -4,7 +4,7 @@ //! Conforms to RustGIS zero-panic architecture: bounded allocations, typed error handling, //! preservation of target ordering, schema preservation, and deterministic collision handling. -use rustgis_data::{Dataset, Field, Value}; +use rustgis_data::{Dataset, Field, FieldType, Value}; use std::collections::HashMap; use std::fmt::Write; @@ -68,6 +68,15 @@ impl JoinKey { } } + /// Extracts a [`JoinKey`] for the given comparison: under [`KeyComparison::NumberAsText`] a + /// whole number is written out in full (`47157000100`, `-5`), so it equals the same digits as text. + pub(crate) fn from_value_compared(v: &Value, comparison: KeyComparison) -> Option { + match (comparison, Self::from_value(v)?) { + (KeyComparison::NumberAsText, JoinKey::Int(i)) => Some(JoinKey::Text(i.to_string())), + (_, key) => Some(key), + } + } + /// User-friendly display representation of the key for error messages. pub fn display(&self) -> String { match self { @@ -79,15 +88,31 @@ impl JoinKey { } } -/// Validates that target key and source key fields have compatible data types. -pub fn validate_key_compatibility(target_field: &Field, source_field: &Field) -> Result<(), String> { +/// How the values of two compatible key fields are compared. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum KeyComparison { + /// Same type, or both numeric: values compared as they are. + Exact, + /// A Text key against an Integer key (an all-digit id typed as a number in one file and as + /// text in the other): the numbers are written out in full and compared with the text exactly, + /// so leading zeros stay meaningful (`"01001"` does not match `1001`). This follows the values, + /// not the side they come from: a whole number on either side (also a whole `Float` that a + /// SQLite-backed Integer column can hold) is compared as its text. + NumberAsText, +} + +/// How the values of the target and source key fields are compared, or why they can't be. +pub(crate) fn key_comparison(target_field: &Field, source_field: &Field) -> Result { let t_ty = target_field.ty; let s_ty = source_field.ty; if t_ty == s_ty || (t_ty.is_numeric() && s_ty.is_numeric()) { - Ok(()) + Ok(KeyComparison::Exact) + } else if matches!((t_ty, s_ty), (FieldType::Text, FieldType::Integer) | (FieldType::Integer, FieldType::Text)) { + Ok(KeyComparison::NumberAsText) } else { Err(format!( - "key field type mismatch: target field '{}' is {}, but source field '{}' is {}", + "key field type mismatch: target field '{}' is {}, but source field '{}' is {}; \ + convert one key to the other's type with Add Field and Calculate Field", target_field.name, t_ty.label(), source_field.name, @@ -112,14 +137,21 @@ pub struct JoinFieldOptions<'a> { pub output_name: String, } -/// Result metadata from a successful Join Field execution. +/// Result metadata from a successful Join Field execution. Only `join_field` builds one, so +/// it is `non_exhaustive`: a new count can be added without breaking callers. #[derive(Clone, Debug, PartialEq, Eq)] +#[non_exhaustive] pub struct JoinFieldReport { pub matched_records: usize, pub unmatched_records: usize, pub total_records: usize, pub transferred_fields: usize, pub duplicate_keys: usize, + /// Unmatched target records whose key equals a source key once leading zeros are dropped + /// (`"01001"` against `1001`): compared as written, they don't match. + pub leading_zero_mismatches: usize, + /// The first such pair as written, target key then source key (`"01001"`, `1001`). + pub leading_zero_example: Option<(String, String)>, } impl JoinFieldReport { @@ -133,6 +165,31 @@ impl JoinFieldReport { } msg } + + /// A warning when records were left unmatched only by leading zeros, or `None`. + pub fn leading_zero_notice(&self) -> Option { + let (target_key, source_key) = self.leading_zero_example.as_ref().filter(|_| self.leading_zero_mismatches > 0)?; + Some(format!( + "Warning: {} record(s) left unmatched because one key has leading zeros the other lacks \ + (first: target key {target_key}, source key {source_key}); keys are compared as written, \ + so give both the same digits, e.g. as text with the zeros kept", + self.leading_zero_mismatches + )) + } +} + +/// An all-digit text key with leading zeros, without them (`"01001"` → `"1001"`, `"00"` → `"0"`). +fn without_leading_zeros(s: &str) -> Option<&str> { + if s.len() < 2 || !s.starts_with('0') || !s.bytes().all(|b| b.is_ascii_digit()) { + return None; + } + let rest = s.trim_start_matches('0'); + Some(if rest.is_empty() { "0" } else { rest }) +} + +/// A key value as the user wrote it: text quoted, numbers bare. +fn written(v: &Value) -> String { + JoinKey::from_value(v).map(|k| k.display()).unwrap_or_default() } /// Executes a non-spatial attribute join from `source` onto `target`. @@ -156,7 +213,7 @@ pub fn join_field(target: &Dataset, source: &Dataset, opts: &JoinFieldOptions<'_ let source_key_field = source.fields.get(source_key_idx).ok_or_else(|| "source key field missing".to_string())?; // 2. Validate key type compatibility - validate_key_compatibility(target_key_field, source_key_field)?; + let comparison = key_comparison(target_key_field, source_key_field)?; // 3. Resolve which source fields to transfer let transfer_indices: Vec = match opts.fields_to_transfer { @@ -214,7 +271,7 @@ pub fn join_field(target: &Dataset, source: &Dataset, opts: &JoinFieldOptions<'_ for (s_idx, s_feat) in source.features.iter().enumerate() { let val = s_feat.attrs.get(source_key_idx).unwrap_or(&Value::Null); - let Some(key) = JoinKey::from_value(val) else { + let Some(key) = JoinKey::from_value_compared(val, comparison) else { // Null or non-finite keys in source cannot match continue; }; @@ -238,6 +295,22 @@ pub fn join_field(target: &Dataset, source: &Dataset, opts: &JoinFieldOptions<'_ } } + // Source text keys with leading zeros, by their digits without them: a target number that + // lost its zeros (`1001` for `"01001"`) doesn't match, and the report says so. + let mut zero_padded_sources: HashMap = HashMap::new(); + if comparison == KeyComparison::NumberAsText { + for (s_idx, s_feat) in source.features.iter().enumerate() { + if let Some(Value::Text(text)) = s_feat.attrs.get(source_key_idx) + && let Some(digits) = without_leading_zeros(text) + { + zero_padded_sources.entry(digits.to_string()).or_insert(s_idx); + } + } + } + let source_key_value = |s_idx: usize| source.features.get(s_idx).and_then(|f| f.attrs.get(source_key_idx)).unwrap_or(&Value::Null); + let mut leading_zero_mismatches = 0usize; + let mut leading_zero_example = None; + // 6. Join records into target let target_len = target.features.len(); let mut matched_records = 0usize; @@ -260,7 +333,22 @@ pub fn join_field(target: &Dataset, source: &Dataset, opts: &JoinFieldOptions<'_ new_attrs.reserve(transfer_indices.len()); let t_val = t_feat.attrs.get(target_key_idx).unwrap_or(&Value::Null); - let matching_s_idx = JoinKey::from_value(t_val).and_then(|k| source_index.get(&k).copied()); + let t_key = JoinKey::from_value_compared(t_val, comparison); + let matching_s_idx = t_key.as_ref().and_then(|k| source_index.get(k).copied()); + if matching_s_idx.is_none() + && comparison == KeyComparison::NumberAsText + && let Some(JoinKey::Text(text)) = &t_key + { + // A target text key with leading zeros against a source key without them, or a target + // key without them against a source text key with them. + let near = without_leading_zeros(text) + .and_then(|digits| source_index.get(&JoinKey::Text(digits.to_string())).copied()) + .or_else(|| zero_padded_sources.get(text).copied()); + if let Some(s_idx) = near { + leading_zero_mismatches += 1; + leading_zero_example.get_or_insert_with(|| (written(t_val), written(source_key_value(s_idx)))); + } + } if let Some(s_idx) = matching_s_idx { matched_records += 1; @@ -287,6 +375,8 @@ pub fn join_field(target: &Dataset, source: &Dataset, opts: &JoinFieldOptions<'_ total_records: target_len, transferred_fields: transfer_indices.len(), duplicate_keys: duplicate_count, + leading_zero_mismatches, + leading_zero_example, }; Ok((out, report)) @@ -538,7 +628,7 @@ mod tests { #[test] fn test_incompatible_key_types_error() { let target = make_table("target", vec![("key", FieldType::Text)], vec![]); - let source = make_table("source", vec![("key", FieldType::Integer)], vec![]); + let source = make_table("source", vec![("key", FieldType::Boolean)], vec![]); let opts = JoinFieldOptions { target_field: "key", join_field: "key", @@ -549,6 +639,300 @@ mod tests { }; let err = join_field(&target, &source, &opts).unwrap_err(); assert!(err.contains("key field type mismatch")); + assert!(err.contains("Add Field and Calculate Field"), "{err}"); + } + + fn key_opts(duplicate_strategy: DuplicateStrategy) -> JoinFieldOptions<'static> { + JoinFieldOptions { + target_field: "GEOID", + join_field: "GEOID", + fields_to_transfer: None, + prefix: "", + duplicate_strategy, + output_name: "out".into(), + } + } + + #[test] + fn test_text_key_matches_integer_key_both_ways() { + // TIGER tracts store GEOID as text; the Census Gazetteer CSV reads it as a number. + let tracts = make_table( + "tracts", + vec![("GEOID", FieldType::Text)], + vec![vec![Value::Text("47157000100".into())], vec![Value::Text("47157000200".into())]], + ); + let gazetteer = make_table( + "gazetteer", + vec![("GEOID", FieldType::Integer), ("ALAND", FieldType::Integer)], + vec![vec![Value::Int(47157000200), Value::Int(2)], vec![Value::Int(47157000100), Value::Int(1)]], + ); + let (out, rep) = join_field(&tracts, &gazetteer, &key_opts(DuplicateStrategy::First)).unwrap(); + assert_eq!(rep.matched_records, 2); + assert_eq!(out.features[0].attrs[1], Value::Int(1)); + assert_eq!(out.features[1].attrs[1], Value::Int(2)); + // The key fields keep their own types. + assert_eq!(out.fields[0].ty, FieldType::Text); + + let (out, rep) = join_field(&gazetteer, &tracts, &key_opts(DuplicateStrategy::First)).unwrap(); + assert_eq!(rep.matched_records, 2); + assert_eq!(out.fields[0].ty, FieldType::Integer); + assert_eq!(out.features[0].attrs[0], Value::Int(47157000200)); + } + + #[test] + fn test_number_as_text_is_exact() { + let target = make_table( + "target", + vec![("GEOID", FieldType::Text)], + vec![ + vec![Value::Text("01001".into())], + vec![Value::Text("1001".into())], + vec![Value::Text("-5".into())], + vec![Value::Text(" 7".into())], + vec![Value::Text("7.0".into())], + vec![Value::Null], + ], + ); + let source = make_table( + "source", + vec![("GEOID", FieldType::Integer), ("tag", FieldType::Text)], + vec![ + vec![Value::Int(1001), Value::Text("county".into())], + vec![Value::Int(-5), Value::Text("negative".into())], + vec![Value::Int(7), Value::Text("seven".into())], + vec![Value::Null, Value::Text("null".into())], + ], + ); + let (out, rep) = join_field(&target, &source, &key_opts(DuplicateStrategy::First)).unwrap(); + let tags: Vec<&Value> = out.features.iter().map(|f| &f.attrs[1]).collect(); + // Leading zeros stay meaningful, text isn't trimmed or parsed, and nulls never match. + assert_eq!( + tags, + [&Value::Null, &Value::Text("county".into()), &Value::Text("negative".into()), &Value::Null, &Value::Null, &Value::Null] + ); + assert_eq!(rep.matched_records, 2); + // Only "01001" is a leading-zero miss; " 7" and "7.0" aren't digits, and nulls aren't keys. + assert_eq!(rep.leading_zero_mismatches, 1); + assert_eq!(rep.leading_zero_example, Some(("\"01001\"".into(), "1001".into()))); + let notice = rep.leading_zero_notice().unwrap(); + assert!(notice.contains("1 record(s) left unmatched"), "{notice}"); + assert!(notice.contains("target key \"01001\", source key 1001"), "{notice}"); + } + + #[test] + fn test_leading_zero_misses_are_reported_both_ways() { + // Target numbers that lost their zeros against source text that kept them. + let target = make_table( + "target", + vec![("GEOID", FieldType::Integer)], + vec![vec![Value::Int(1001)], vec![Value::Int(7)], vec![Value::Int(0)], vec![Value::Int(42)], vec![Value::Int(5)]], + ); + let source = make_table( + "source", + vec![("GEOID", FieldType::Text), ("tag", FieldType::Text)], + vec![ + vec![Value::Text("01001".into()), Value::Text("county".into())], + vec![Value::Text("007".into()), Value::Text("seven".into())], + vec![Value::Text("00".into()), Value::Text("zero".into())], + vec![Value::Text("42".into()), Value::Text("answer".into())], + vec![Value::Text("0x5".into()), Value::Text("hex".into())], + ], + ); + let (out, rep) = join_field(&target, &source, &key_opts(DuplicateStrategy::First)).unwrap(); + assert_eq!(out.features.iter().filter(|f| f.attrs[1] != Value::Null).count(), 1, "only 42 matches"); + assert_eq!((rep.matched_records, rep.leading_zero_mismatches), (1, 3)); + assert_eq!(rep.leading_zero_example, Some(("1001".into(), "\"01001\"".into()))); + assert!(rep.leading_zero_notice().unwrap().contains("3 record(s) left unmatched")); + + // The same tables the other way round: target text with zeros against source numbers. + let (_, rep) = join_field(&source, &target, &key_opts(DuplicateStrategy::First)).unwrap(); + assert_eq!((rep.matched_records, rep.leading_zero_mismatches), (1, 3)); + assert_eq!(rep.leading_zero_example, Some(("\"01001\"".into(), "1001".into()))); + } + + #[test] + fn test_no_leading_zero_notice_without_a_miss() { + // Every key matches, or misses with no near match: no notice. + let target = make_table( + "target", + vec![("GEOID", FieldType::Text)], + vec![vec![Value::Text("0100".into())], vec![Value::Text("0200".into())]], + ); + let source = make_table( + "source", + vec![("GEOID", FieldType::Integer), ("tag", FieldType::Text)], + vec![vec![Value::Int(100), Value::Text("hundred".into())], vec![Value::Int(300), Value::Text("other".into())]], + ); + let (_, rep) = join_field(&target, &source, &key_opts(DuplicateStrategy::First)).unwrap(); + assert_eq!((rep.matched_records, rep.leading_zero_mismatches), (0, 1), "premise: \"0100\" is a miss, \"0200\" isn't near"); + let clean = + make_table("clean", vec![("GEOID", FieldType::Text)], vec![vec![Value::Text("100".into())], vec![Value::Text("200".into())]]); + let (_, rep) = join_field(&clean, &source, &key_opts(DuplicateStrategy::First)).unwrap(); + assert_eq!((rep.matched_records, rep.leading_zero_mismatches, &rep.leading_zero_example), (1, 0, &None)); + assert_eq!(rep.leading_zero_notice(), None); + // Same-type text keys are compared exactly, as before, and aren't second-guessed. + let text_source = make_table("ts", vec![("GEOID", FieldType::Text)], vec![vec![Value::Text("100".into())]]); + let (_, rep) = join_field(&target, &text_source, &key_opts(DuplicateStrategy::First)).unwrap(); + assert_eq!((rep.matched_records, rep.leading_zero_mismatches), (0, 0)); + } + + #[test] + fn test_without_leading_zeros() { + assert_eq!(without_leading_zeros("01001"), Some("1001")); + assert_eq!(without_leading_zeros("00"), Some("0")); + assert_eq!(without_leading_zeros("000120"), Some("120")); + for s in ["", "0", "1001", "10", "0x5", "0 1", "-01", "01.0", "01"] { + assert_eq!(without_leading_zeros(s), None, "{s:?}"); + } + } + + #[test] + fn test_number_as_text_duplicate_keys() { + let target = make_table("target", vec![("GEOID", FieldType::Text)], vec![vec![Value::Text("42".into())]]); + let source = make_table( + "source", + vec![("GEOID", FieldType::Integer), ("tag", FieldType::Text)], + vec![vec![Value::Int(42), Value::Text("first".into())], vec![Value::Int(42), Value::Text("last".into())]], + ); + let (out, rep) = join_field(&target, &source, &key_opts(DuplicateStrategy::Last)).unwrap(); + assert_eq!(out.features[0].attrs[1], Value::Text("last".into())); + assert_eq!(rep.duplicate_keys, 1); + let err = join_field(&target, &source, &key_opts(DuplicateStrategy::Error)).unwrap_err(); + assert!(err.contains("duplicate key \"42\""), "{err}"); + } + + #[test] + fn test_text_key_against_double_key_is_refused() { + let target = make_table("target", vec![("GEOID", FieldType::Text)], vec![]); + let source = make_table("source", vec![("GEOID", FieldType::Double)], vec![]); + let err = join_field(&target, &source, &key_opts(DuplicateStrategy::First)).unwrap_err(); + assert!(err.contains("key field type mismatch"), "{err}"); + } + + #[test] + fn test_key_type_compatibility_matrix() { + use FieldType::*; + use KeyComparison::*; + let types = [Integer, Double, Text, Boolean]; + for t in types { + for s in types { + let expected = match (t, s) { + _ if t == s => Some(Exact), + (Integer, Double) | (Double, Integer) => Some(Exact), + (Text, Integer) | (Integer, Text) => Some(NumberAsText), + _ => None, + }; + let (a, b) = (Field::new("a", t), Field::new("b", s)); + let got = key_comparison(&a, &b); + assert_eq!(got.as_ref().ok().copied(), expected, "{t:?} against {s:?}: {got:?}"); + if let Err(e) = got { + assert!(e.contains(t.label()) && e.contains(s.label()) && e.contains("Add Field and Calculate Field"), "{e}"); + } + } + } + } + + /// Joins one text key against one integer key, both ways round, and says whether they matched. + fn text_matches_int(text: &str, int: i64) -> bool { + let text_table = make_table( + "t", + vec![("GEOID", FieldType::Text), ("tag", FieldType::Text)], + vec![vec![Value::Text(text.into()), Value::Text("text side".into())]], + ); + let int_table = make_table( + "i", + vec![("GEOID", FieldType::Integer), ("tag", FieldType::Text)], + vec![vec![Value::Int(int), Value::Text("int side".into())]], + ); + let (_, a) = join_field(&text_table, &int_table, &key_opts(DuplicateStrategy::First)).unwrap(); + let (_, b) = join_field(&int_table, &text_table, &key_opts(DuplicateStrategy::First)).unwrap(); + assert_eq!(a.matched_records, b.matched_records, "{text:?} against {int} differs by direction"); + a.matched_records == 1 + } + + #[test] + fn test_number_as_text_round_trips_every_magnitude() { + let mut values = vec![0, 1, -1, 9, 10, 99, 100, 1001, 47157000100, i64::MAX, i64::MIN, i64::MAX - 1, i64::MIN + 1]; + let mut p = 1i64; + while let Some(next) = p.checked_mul(10) { + values.extend([p - 1, p, p + 1, -p]); + p = next; + } + for n in values { + assert!(text_matches_int(&n.to_string(), n), "{n}"); + // Spellings of the same number that are not the same text never match. + for other in [format!("0{n}"), format!("+{n}"), format!(" {n}"), format!("{n} "), format!("{n}.0")] { + assert!(!text_matches_int(&other, n), "{other:?} matched {n}"); + } + } + } + + #[test] + fn test_number_as_text_rejects_look_alikes() { + for (text, n) in [ + ("-0", 0), + ("00", 0), + ("4_2", 42), + ("42e0", 42), + ("0x2A", 42), + ("4,2", 42), + ("42", 42), // full-width digits + ("٤٢", 42), // Arabic-Indic digits + ("42\u{200b}", 42), + ("", 0), + ("9223372036854775808", i64::MIN), // past i64::MAX, not a wrap-around + ] { + assert!(!text_matches_int(text, n), "{text:?} matched {n}"); + } + } + + #[test] + fn test_number_as_text_compares_values_not_declared_types() { + // Rows can hold values that disagree with their field's type (a hostile or hand-built + // file): the comparison follows the value, and nothing panics. + let target = make_table( + "target", + vec![("GEOID", FieldType::Integer)], + vec![ + vec![Value::Float(42.0)], + vec![Value::Float(42.5)], + vec![Value::Float(f64::NAN)], + vec![Value::Text("7".into())], + vec![Value::Bool(true)], + ], + ); + let source = make_table( + "source", + vec![("GEOID", FieldType::Text), ("tag", FieldType::Text)], + vec![ + vec![Value::Text("42".into()), Value::Text("forty-two".into())], + vec![Value::Text("42.5".into()), Value::Text("fraction".into())], + vec![Value::Text("NaN".into()), Value::Text("nan".into())], + vec![Value::Int(7), Value::Text("seven".into())], + vec![Value::Text("true".into()), Value::Text("bool".into())], + ], + ); + let (out, rep) = join_field(&target, &source, &key_opts(DuplicateStrategy::Error)).unwrap(); + let tags: Vec<&Value> = out.features.iter().map(|f| &f.attrs[1]).collect(); + assert_eq!(tags, [&Value::Text("forty-two".into()), &Value::Null, &Value::Null, &Value::Text("seven".into()), &Value::Null]); + assert_eq!((rep.matched_records, rep.duplicate_keys), (2, 0)); + } + + #[test] + fn test_exact_comparison_is_unchanged_for_same_type_keys() { + // Text against text stays exact: no number parsing sneaks in. + let target = + make_table("target", vec![("GEOID", FieldType::Text)], vec![vec![Value::Text("042".into())], vec![Value::Text("42".into())]]); + let source = make_table( + "source", + vec![("GEOID", FieldType::Text), ("tag", FieldType::Text)], + vec![vec![Value::Text("42".into()), Value::Text("hit".into())]], + ); + let (out, _) = join_field(&target, &source, &key_opts(DuplicateStrategy::First)).unwrap(); + assert_eq!(out.features[0].attrs[1], Value::Null); + assert_eq!(out.features[1].attrs[1], Value::Text("hit".into())); + assert_eq!(JoinKey::from_value_compared(&Value::Int(5), KeyComparison::Exact), Some(JoinKey::Int(5))); + assert_eq!(JoinKey::from_value_compared(&Value::Null, KeyComparison::NumberAsText), None); } #[test] diff --git a/crates/analysis/src/tools.rs b/crates/analysis/src/tools.rs index 70ca3ff70..e8cc2bd8c 100644 --- a/crates/analysis/src/tools.rs +++ b/crates/analysis/src/tools.rs @@ -4102,6 +4102,7 @@ fn join_field_tool(inp: &dyn Inputs, p: &Json) -> Result { let (out, report) = crate::attribute_join::join_field(&target_ref, &source_ref, &opts)?; messages.push(report.message(&target.name)); + messages.extend(report.leading_zero_notice()); let output = if out.geometry_type == rustgis_data::GeometryType::None { Output::Table(out) } else { Output::Layer(out) }; @@ -8131,19 +8132,21 @@ mod tests { .unwrap_err(); assert!(err_dup.contains("duplicate key")); - // 4. Incompatible key type error - let mut int_source = Dataset::new("int_source", GeometryType::None, vec![Field::new("num_key", FieldType::Integer)]); - int_source.push(None, vec![Value::Int(123)]); - let m4 = - Map(HashMap::from([("inventory".to_string(), m2.0.get("inventory").unwrap().clone()), ("int_source".to_string(), int_source)])); + // 4. Incompatible key type error (a text key matches an integer key, not a boolean one) + let mut bool_source = Dataset::new("bool_source", GeometryType::None, vec![Field::new("flag_key", FieldType::Boolean)]); + bool_source.push(None, vec![Value::Bool(true)]); + let m4 = Map(HashMap::from([ + ("inventory".to_string(), m2.0.get("inventory").unwrap().clone()), + ("bool_source".to_string(), bool_source), + ])); let err_type = crate::run( "joinField", &m4, &serde_json::json!({ "target": "inventory", "targetField": "sku", - "join": "int_source", - "joinField": "num_key" + "join": "bool_source", + "joinField": "flag_key" }), ) .unwrap_err(); diff --git a/crates/analysis/tests/join_field.rs b/crates/analysis/tests/join_field.rs new file mode 100644 index 000000000..6a798b2be --- /dev/null +++ b/crates/analysis/tests/join_field.rs @@ -0,0 +1,170 @@ +//! Join Field through the tool, on tables read by the real CSV and Shapefile readers: an all-digit +//! id that one file types as a number and the other as text still joins (#1178). +use rustgis_analysis::{Inputs, Output}; +use rustgis_data::io::{read_csv, read_path, write_path}; +use rustgis_data::{Coord, Dataset, Field, FieldType, Geometry, GeometryType, Value}; +use serde_json::json; +use std::collections::HashMap; +use std::sync::atomic::{AtomicUsize, Ordering}; + +#[derive(Default)] +struct Layers { + data: HashMap, + selected: HashMap>, +} +impl Inputs for Layers { + fn dataset(&self, name: &str) -> Option<&Dataset> { + self.data.get(name) + } + fn selection(&self, name: &str) -> &[usize] { + self.selected.get(name).map(Vec::as_slice).unwrap_or(&[]) + } +} +impl Layers { + fn with(mut self, ds: Dataset) -> Self { + self.data.insert(ds.name.clone(), ds); + self + } +} + +fn field_ty(ds: &Dataset, name: &str) -> FieldType { + ds.fields.iter().find(|f| f.name == name).map(|f| f.ty).unwrap() +} +fn column<'a>(ds: &'a Dataset, name: &str) -> Vec<&'a Value> { + let i = ds.fields.iter().position(|f| f.name == name).unwrap(); + ds.features.iter().map(|f| &f.attrs[i]).collect() +} + +/// Census tracts as a TIGER/Line shapefile holds them (`GEOID` a character field), written and +/// read back through the Shapefile reader. +fn tracts() -> Dataset { + static CALLS: AtomicUsize = AtomicUsize::new(0); + let mut ds = + Dataset::new("tracts", GeometryType::Point, vec![Field::new("GEOID", FieldType::Text), Field::new("NAMELSAD", FieldType::Text)]); + for (geoid, x) in [("47157000100", -90.05), ("47157000200", -90.04), ("47157000300", -90.03), ("47157980100", -90.02)] { + ds.push_next(Some(Geometry::Point(Coord::new(x, 35.1))), vec![Value::Text(geoid.into()), Value::Text(format!("Tract {geoid}"))]); + } + let dir = std::env::temp_dir().join(format!("rustgis-join-field-{}-{}", std::process::id(), CALLS.fetch_add(1, Ordering::Relaxed))); + std::fs::create_dir_all(&dir).unwrap(); + write_path(&ds, &dir.join("tracts.shp")).unwrap(); + let (read, _) = read_path(&dir.join("tracts.shp")).unwrap(); + std::fs::remove_dir_all(&dir).unwrap(); + assert_eq!(read.name, "tracts"); + assert_eq!(field_ty(&read, "GEOID"), FieldType::Text, "premise: the Shapefile reader keeps an all-digit character GEOID as text"); + read +} + +/// The Census Gazetteer's tract table as a CSV: the reader types an all-digit `GEOID` as a number. +fn gazetteer() -> Dataset { + let csv = "GEOID,ALAND,AWATER\n47157000300,3000,30\n47157000100,1000,10\n47157000200,2000,20\n47157999999,9,0\n"; + let (ds, _) = read_csv(csv, "gazetteer").unwrap(); + assert_eq!(field_ty(&ds, "GEOID"), FieldType::Integer, "premise: the CSV reader types GEOID as Integer"); + ds +} + +#[test] +fn census_gazetteer_joins_tiger_tracts_by_geoid() { + let layers = Layers::default().with(tracts()).with(gazetteer()); + let res = + rustgis_analysis::run("joinField", &layers, &json!({"target":"tracts","targetField":"GEOID","joinTable":"gazetteer"})).unwrap(); + let Output::Layer(out) = res.output else { panic!("a layer target gives a layer") }; + assert_eq!(column(&out, "ALAND"), [&Value::Int(1000), &Value::Int(2000), &Value::Int(3000), &Value::Null]); + assert_eq!(column(&out, "AWATER"), [&Value::Int(10), &Value::Int(20), &Value::Int(30), &Value::Null]); + // The target's key, rows, ids and geometry are kept as they were. + let before = tracts(); + assert_eq!(field_ty(&out, "GEOID"), FieldType::Text); + assert_eq!(column(&out, "GEOID"), column(&before, "GEOID")); + assert_eq!(out.features.iter().map(|f| f.id).collect::>(), before.features.iter().map(|f| f.id).collect::>()); + assert!(out.features.iter().zip(before.features.iter()).all(|(a, b)| a.geometry == b.geometry)); + assert!(res.messages.iter().any(|m| m.contains("3/4 records matched")), "{:?}", res.messages); + assert!(!res.messages.iter().any(|m| m.contains("leading zeros")), "no zeros were lost: {:?}", res.messages); +} + +#[test] +fn a_number_key_target_joins_a_text_key_source() { + let layers = Layers::default().with(tracts()).with(gazetteer()); + let res = rustgis_analysis::run( + "joinField", + &layers, + &json!({"target":"gazetteer","targetField":"GEOID","joinTable":"tracts","fields":["NAMELSAD"]}), + ) + .unwrap(); + let Output::Table(out) = res.output else { panic!("a table target gives a table") }; + assert_eq!(field_ty(&out, "GEOID"), FieldType::Integer, "the target key keeps its type"); + assert_eq!(field_ty(&out, "NAMELSAD"), FieldType::Text); + assert_eq!( + column(&out, "NAMELSAD"), + [ + &Value::Text("Tract 47157000300".into()), + &Value::Text("Tract 47157000100".into()), + &Value::Text("Tract 47157000200".into()), + &Value::Null + ] + ); +} + +#[test] +fn codes_with_leading_zeros_only_match_as_written() { + // The CSV reader keeps `01001` as text; a layer stores the same county as the number 1001. + let (codes, _) = read_csv("FIPS,county\n01001,Autauga\n01003,Baldwin\n1005,Barbour\n", "codes").unwrap(); + assert_eq!(field_ty(&codes, "FIPS"), FieldType::Text, "premise: a leading zero keeps the column text"); + let (counties, _) = read_csv("FIPS,pop\n1001,58805\n1003,231767\n1005,25223\n", "counties").unwrap(); + assert_eq!(field_ty(&counties, "FIPS"), FieldType::Integer); + let layers = Layers::default().with(codes).with(counties); + let res = rustgis_analysis::run("joinField", &layers, &json!({"target":"counties","targetField":"FIPS","joinTable":"codes"})).unwrap(); + let Output::Table(out) = res.output else { panic!("a table target gives a table") }; + // No leading zeros are made up: only the code written without one matches. + assert_eq!(column(&out, "county"), [&Value::Null, &Value::Null, &Value::Text("Barbour".into())]); + assert!(res.messages.iter().any(|m| m.contains("1/3 records matched")), "{:?}", res.messages); + // The two codes that lost their zero are named, not only counted. + let notice = res.messages.iter().find(|m| m.contains("leading zeros")).unwrap_or_else(|| panic!("{:?}", res.messages)); + assert!(notice.contains("2 record(s) left unmatched"), "{notice}"); + assert!(notice.contains("target key 1001, source key \"01001\""), "{notice}"); +} + +#[test] +fn selections_are_honoured_with_mixed_key_types() { + let mut layers = Layers::default().with(tracts()).with(gazetteer()); + layers.selected.insert("tracts".into(), vec![0, 1, 3]); + layers.selected.insert("gazetteer".into(), vec![0, 1]); + let res = + rustgis_analysis::run("joinField", &layers, &json!({"target":"tracts","targetField":"GEOID","joinTable":"gazetteer"})).unwrap(); + let Output::Layer(out) = res.output else { panic!("a layer target gives a layer") }; + assert_eq!( + column(&out, "GEOID"), + [&Value::Text("47157000100".into()), &Value::Text("47157000200".into()), &Value::Text("47157980100".into())] + ); + // 47157000100 matches a selected record; 47157000200 is in the gazetteer but not among its + // selected records. + assert_eq!(column(&out, "ALAND"), [&Value::Int(1000), &Value::Null, &Value::Null]); +} + +#[test] +fn duplicate_strategies_apply_to_mixed_key_types() { + let (dups, _) = read_csv("GEOID,ALAND\n47157000100,1\n47157000100,2\n", "dups").unwrap(); + let layers = Layers::default().with(tracts()).with(dups); + let run = |duplicates: &str| { + rustgis_analysis::run( + "joinField", + &layers, + &json!({"target":"tracts","targetField":"GEOID","joinTable":"dups","duplicates":duplicates}), + ) + }; + let Output::Layer(first) = run("First").unwrap().output else { panic!() }; + assert_eq!(column(&first, "ALAND")[0], &Value::Int(1)); + let Output::Layer(last) = run("Last").unwrap().output else { panic!() }; + assert_eq!(column(&last, "ALAND")[0], &Value::Int(2)); + let err = run("Error").err().unwrap(); + assert!(err.contains("duplicate key \"47157000100\""), "{err}"); +} + +#[test] +fn other_key_type_pairs_are_refused_with_the_way_out() { + let (decimal, _) = read_csv("GEOID,v\n47157000100.5,1\n", "decimal").unwrap(); + assert_eq!(field_ty(&decimal, "GEOID"), FieldType::Double); + let layers = Layers::default().with(tracts()).with(decimal); + let err = + rustgis_analysis::run("joinField", &layers, &json!({"target":"tracts","targetField":"GEOID","joinTable":"decimal"})).err().unwrap(); + assert!(err.contains("target field 'GEOID' is Text, but source field 'GEOID' is Double"), "{err}"); + assert!(err.contains("Add Field and Calculate Field"), "{err}"); +} diff --git a/docs/user-guide/analysis.md b/docs/user-guide/analysis.md index ead5f7783..1ec528b00 100644 --- a/docs/user-guide/analysis.md +++ b/docs/user-guide/analysis.md @@ -411,7 +411,13 @@ into a `target` table or layer based on matching key values: `joinField` (aliases: `join_field`) names the matching key in the source table; when omitted, empty, or whitespace-only, it defaults to `targetField`. Numeric keys match across integer and floating-point types when whole numbers (e.g. `10` matches `10.0`), while non-finite floats (`NaN`, infinity) and - nulls never match. Incompatible key data types (e.g. text vs integer) are rejected. + nulls never match. A text key matches an integer key when the text is the number written out in + full (`"47157000100"` matches `47157000100`), so an all-digit ID typed as a number in one table and + as text in the other (Census GEOIDs, FIPS codes, ZIP codes) joins when the codes match exactly as + written. Leading zeros stay meaningful: a number that lost its zero (`1001` for `"01001"`) leaves its + record unmatched, and a warning says how many records missed this way, with the first pair. Other + incompatible key data types (e.g. text vs floating point or boolean) are rejected, and the message + suggests converting one key with Add Field and Calculate Field. - **Transferred fields:** `fields` selects specific source fields to transfer (a comma-separated list or a JSON array of strings, e.g. `["tax_rate", "zone_code"]`; non-string and non-array types are rejected). Repeated field names are deduplicated in order. When omitted or passed as a blank string (`""`), every source field except