fix(import): refuse a raw date spelled like a generated item or group (improvise-d0n6)
This commit is contained in:
committed by
Edward Langley
parent
b516567e58
commit
c3e12842c2
@@ -638,7 +638,15 @@ ones beside it (`Date_Year`, `Date_Quarter`, `Date_Month`):
|
||||
ungrouped item, with a warning naming the column and the row count, rather
|
||||
than being dropped. The warning is separate from the split shape's, because
|
||||
that one reports an absent coordinate and this one a coordinate that is there
|
||||
and ungrouped.
|
||||
and ungrouped. **Unless its spelling is one the hierarchy generates**
|
||||
(improvise-d0n6): a raw `2024-01` or `2024` is the same name as the generated
|
||||
month or year, so it would join the grouped item and the warning would be
|
||||
false. The build **refuses**, naming the column and the spellings and pointing
|
||||
at the split shape. That follows the collision rule below: where two records
|
||||
would silently be one thing, the user is asked and nothing is defaulted. A
|
||||
fallback to the split shape was rejected as a silent change of the user's
|
||||
choice, and a renamed raw item as altering the data. The check is
|
||||
`Plan::spelling_clash`.
|
||||
- **Item order is first-seen, not chronological.** Order is a correctness
|
||||
property here (gotcha 18), and a group need not be contiguous, so a CSV that
|
||||
is not in date order still nests correctly. It is just drawn in the order
|
||||
|
||||
@@ -1904,3 +1904,18 @@ improvise-8jk6.12 (4). A `region` of `" "` is kept as a record with no
|
||||
predates this PR (improvise-ych), but the sparse-record rule now rests on it:
|
||||
without it the value becomes an item named `" "`. Rival F4 (the reviewer's
|
||||
fault) fails it.
|
||||
|
||||
### a_raw_date_spelled_like_a_generated_item_is_refused
|
||||
|
||||
improvise-d0n6 (Codex P2 on PR #322). A `%Y-%m-%d` column imported Year > Month
|
||||
with `2024-01-15` and a raw `2024-01`. Before the refusal the build succeeded,
|
||||
put both records on the grouped item `2024-01` (summing them) and warned that
|
||||
the raw value was "kept ungrouped", which was false. Fault injected: removing
|
||||
the `spelling_clash` call makes this test fail with `unwrap_err()` on an `Ok`.
|
||||
|
||||
### a_raw_date_spelled_like_a_generated_group_is_refused
|
||||
|
||||
improvise-d0n6. The same with a raw `2024`, a generated group. Removing the
|
||||
`spelling_clash` call fails it the same way. The control,
|
||||
`a_raw_date_spelled_like_nothing_generated_is_kept_ungrouped`, passes either
|
||||
way, so a blanket refusal of every unparsed value fails it.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::{HashMap, HashSet};
|
||||
|
||||
use anyhow::{Result, anyhow};
|
||||
use serde_json::Value;
|
||||
@@ -252,6 +252,9 @@ impl ImportPipeline {
|
||||
}
|
||||
|
||||
let placements: Vec<Placement> = self.records.iter().map(|r| plan.place(r)).collect();
|
||||
if let Some(clash) = plan.spelling_clash(&placements) {
|
||||
return Err(anyhow!(clash));
|
||||
}
|
||||
let collisions = collisions_among(&placements);
|
||||
let occurrence = match (&self.on_duplicate, collisions.is_empty()) {
|
||||
(_, true) | (DuplicatePolicy::Sum, false) => None,
|
||||
@@ -617,6 +620,60 @@ impl Plan<'_> {
|
||||
}
|
||||
}
|
||||
|
||||
impl Plan<'_> {
|
||||
/// Why the build stops when a value the date format could not read is spelled
|
||||
/// like an item or group the hierarchy generates (improvise-d0n6). Both would
|
||||
/// be one name in the category, so the raw record would silently join the
|
||||
/// grouped item, and the "ungrouped" warning would be false.
|
||||
fn spelling_clash(&self, placements: &[Placement]) -> Option<String> {
|
||||
let generated = |field: &str| -> HashSet<&str> {
|
||||
placements
|
||||
.iter()
|
||||
.filter_map(|p| match p {
|
||||
Placement::Placed { chains, .. } => Some(chains),
|
||||
Placement::NotObject => None,
|
||||
})
|
||||
.flatten()
|
||||
.filter(|(cat, _)| cat == field)
|
||||
.flat_map(|(_, chain)| chain.iter().map(String::as_str))
|
||||
.collect()
|
||||
};
|
||||
for (at, h) in self.date_hierarchies.iter().enumerate() {
|
||||
let tally = self.date_extractions.len() + at;
|
||||
let names = generated(h.field);
|
||||
let mut clashing: Vec<&str> = vec![];
|
||||
for p in placements {
|
||||
let Placement::Placed {
|
||||
coords, unparsed, ..
|
||||
} = p
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
let raw = coords.iter().find(|(c, _)| c == h.field).map(|(_, v)| v);
|
||||
match raw {
|
||||
Some(v) if unparsed.contains(&tally) && names.contains(v.as_str()) => {
|
||||
if !clashing.contains(&v.as_str()) {
|
||||
clashing.push(v);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
if !clashing.is_empty() {
|
||||
return Some(format!(
|
||||
"'{}' has value(s) that do not match the date format '{}' but are spelled \
|
||||
like a month, quarter or year it generates: {}; fix them, or import the \
|
||||
column with the split shape",
|
||||
h.field,
|
||||
h.format,
|
||||
clashing.join(", ")
|
||||
));
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// One hierarchical date coordinate: its category and its chain, root first.
|
||||
type DateChain = (String, Vec<String>);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user