osm-katana/tests/node_refs_reaches_every_writer.rs
//! **`include_node_refs` reaches the writers from `ConvertOptions`, or it does not
//! exist** — `katana-6` / `katana-14`.
//!
//! ## Why this file
//!
//! The flag itself has been real in `writer.rs` since the way-column consolidation:
//! `ways_schema(include_node_refs)`, `WayColumns::new`, `write_ways_with`,
//! `WayWriter::with_node_refs`, `WayAccumulator::with_node_refs` and
//! `parallel_way_writer_with` all carry it, and `writer.rs`'s own unit tests prove
//! each of them drops the column and only that column. What did NOT exist was the
//! last hop: `grep include_node_refs` over `osm-katana/src` outside `writer.rs` and
//! `shared/schema.rs` returned **zero** hits, so `ConvertOptions` could not set it and
//! the CLI had no flag. 26.5 % of `ways.parquet` — 237.1 MB of korp's 893.3 MB
//! `sweden` cut — was unreachable from every caller in the constellation.
//!
//! `katana-14`'s note is the reason this guard is written the way it is: *"put
//! `include_node_refs` on `ConvertOptions` only when it really reaches every writer;
//! a half-wired flag is worse than none."* A unit test on `WayColumns` cannot see
//! that hop. So this drives the REAL `osm_katana::convert` on a REAL OSM file and
//! reads the APPLIED OUTPUT — the schema of the parquet on disk — not a flag that
//! was round-tripped through a struct.
//!
//! ## What is and is not covered
//!
//! The fixture is `liechtenstein.osm.bz2`, so this drives the **bz2 streaming
//! convert** — the path `ConvertOptions` reaches through `xml_reader`'s streaming
//! sinks, which is where the way writer is constructed. The mmap PBF path shares the
//! same `Tables` value and the same `writer::parallel_way_writer_with` call, but it
//! is NOT exercised here because the crate ships no `.pbf` fixture; that is a stated
//! gap, not a claim.
use std::path::{Path, PathBuf};
fn fixture_bz2() -> PathBuf {
PathBuf::from(concat!(
env!("CARGO_MANIFEST_DIR"),
"/../lbzip2/test_data/liechtenstein.osm.bz2"
))
}
fn opts(dir: &Path, node_refs: bool) -> osm_katana::ConvertOptions {
osm_katana::ConvertOptions {
output_dir: dir.to_path_buf(),
include_node_refs: node_refs,
// Ways are the table under test; nodes come with them because pass 2 resolves
// way geometry through the node store either way. Relations are off so the run
// is as short as it can be while still writing a real `ways.parquet`.
include_rels: false,
keep_node_coords: false,
reuse_node_coords: false,
..Default::default()
}
}
/// `(column names in file order, row count, file bytes)` of a written `ways.parquet`.
fn ways_shape(dir: &Path) -> (Vec<String>, i64, u64) {
let path = dir.join("ways.parquet");
let bytes = std::fs::metadata(&path)
.unwrap_or_else(|e| panic!("stat {path:?}: {e}"))
.len();
let f = std::fs::File::open(&path).unwrap_or_else(|e| panic!("open {path:?}: {e}"));
let b = parquet::arrow::arrow_reader::ParquetRecordBatchReaderBuilder::try_new(f)
.unwrap_or_else(|e| panic!("read {path:?}: {e}"));
let cols: Vec<String> = b
.schema()
.fields()
.iter()
.map(|f| f.name().to_string())
.collect();
let rows = b.metadata().file_metadata().num_rows();
(cols, rows, bytes)
}
/// ★ **RED-when-broken: the flag on `ConvertOptions` decides the shape of the file
/// on disk.**
///
/// Both arms run the same real convert over the same real input and differ in one
/// `bool`. The assertions are on the parquet's own schema and row count — the applied
/// output — because that is the only thing a downstream reader ever sees.
///
/// Three separate properties, because two of them can hold while the third is broken:
///
/// 1. `true` (the default, and what every existing caller and published pack gets)
/// still WRITES `node_refs`. Without this arm the guard would pass on a build that
/// dropped the column unconditionally.
/// 2. `false` OMITS it — absent, not a column of empty lists. A present empty column
/// would claim every way has no nodes, which is the lie `ways_schema`'s own doc
/// forbids.
/// 3. Nothing else moves: the other four columns, in order, and the row count are
/// identical between the arms. A "saving" that also lost ways would be a much worse
/// defect than the one being fixed, and it would look exactly like a success.
///
/// RED SEEN 2026-08-22 on oden, twice, and the FIRST one found a real bug rather than
/// confirming an expectation:
///
/// * The first cut of this fix wired the flag into `parallel_way_writer_with` but left
/// `pass2_accumulate`'s per-worker `WAY_ACC` on `WayAccumulator::new()` — always
/// `true`. Every worker then handed a 5-column batch to a 4-column schema and 32
/// threads died inside arrow with `Field 'version' has type Int32, array has type
/// List(Int64)`, a message that names an innocent column because the pair-by-position
/// zip shifts everything after the missing one. **That is precisely the half-wired
/// flag `katana-14` warned about, and only a guard that runs a real convert and reads
/// the file could see it.** Fixed by construction: the accumulator asks the WRITER
/// (`ColumnEncoder::has_column("node_refs")`) instead of keeping a second copy of the
/// flag (LAW 5), and `encode` now `ensure!`s the column counts agree and says which
/// side turned what off.
/// * Deliberate revert of the three way-writer construction sites back to
/// `WayWriter::new` / `parallel_way_writer` — the flag carried to the writer and then
/// ignored — fails this test at the `convert` call with `the writer's schema has 5
/// column(s) ["id", "geometry", "tags", "node_refs", "version"] but the batch has 4
/// — an optional column was turned off on one side only`. Nothing about the
/// `ConvertOptions` field or the CLI flag changes in that revert, which is exactly
/// why the guard has to read the file and not the flag.
#[test]
fn convert_options_include_node_refs_decides_the_written_ways_schema() {
let with = tempfile::tempdir().unwrap();
let without = tempfile::tempdir().unwrap();
osm_katana::convert(&fixture_bz2(), &opts(with.path(), true)).expect("convert with node_refs");
osm_katana::convert(&fixture_bz2(), &opts(without.path(), false))
.expect("convert without node_refs");
let (cols_with, rows_with, bytes_with) = ways_shape(with.path());
let (cols_without, rows_without, bytes_without) = ways_shape(without.path());
// NON-VACUITY: a fixture that produced no ways would make every assertion below
// trivially true, and "no node_refs column" is what an empty table looks like too.
assert!(
rows_with > 1_000,
"the fixture wrote only {rows_with} ways — too few to be reading a real \
extract, so this guard proves nothing"
);
assert!(
cols_with.iter().any(|c| c == "node_refs"),
"node_refs must be PRESENT in ways.parquet when include_node_refs = true \
(the default, and the shape of every published pack), got {cols_with:?}"
);
assert!(
!cols_without.iter().any(|c| c == "node_refs"),
"node_refs must be ABSENT from ways.parquet when include_node_refs = false, \
got {cols_without:?} — an absent column is honest, a present column of empty \
lists would claim every way has no nodes"
);
let expected: Vec<&str> = cols_with
.iter()
.map(String::as_str)
.filter(|c| *c != "node_refs")
.collect();
let got: Vec<&str> = cols_without.iter().map(String::as_str).collect();
assert_eq!(
got, expected,
"dropping node_refs must not touch any OTHER column or their order"
);
assert_eq!(
rows_without, rows_with,
"dropping node_refs must not lose a single way ({rows_without} vs {rows_with})"
);
let saved = bytes_with.saturating_sub(bytes_without);
let pct = saved as f64 * 100.0 / bytes_with as f64;
assert!(
bytes_without < bytes_with,
"ways.parquet without node_refs ({bytes_without} B) is not smaller than with \
({bytes_with} B) — the column was declared away but its bytes are still \
being written somewhere"
);
eprintln!(
"[katana-6] liechtenstein ways.parquet: {rows_with} ways, \
{bytes_with} B with node_refs, {bytes_without} B without — {saved} B \
saved ({pct:.1} %)"
);
}