Skip to content

Commit 5c975c3

Browse files
jqnatividadclaude
andcommitted
fix(pragmastat): address Copilot review — true ms precision and cache validation
- Increase DAY_DECIMAL_PLACES from 5 to 8 for actual millisecond precision (1ms / 86_400_000 ms-per-day ≈ 1.16e-8) - Round epoch-ms values before i64 cast in fmt_timestamp to avoid truncation - Validate cache record count matches header count in columns_from_cache, ignoring caches generated with --select Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0d85abd commit 5c975c3

File tree

1 file changed

+14
-9
lines changed

1 file changed

+14
-9
lines changed

src/cmd/pragmastat.rs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,9 @@ use crate::{
152152

153153
/// Milliseconds per day — used to convert epoch-ms spreads/shifts to days.
154154
const MS_IN_DAY: f64 = 86_400_000.0;
155-
/// Decimal places for day-valued outputs (5 = millisecond precision).
156-
const DAY_DECIMAL_PLACES: u32 = 5;
155+
/// Decimal places for day-valued outputs (8 ≈ millisecond precision;
156+
/// 1 ms / 86_400_000 ms-per-day ≈ 1.16e-8).
157+
const DAY_DECIMAL_PLACES: u32 = 8;
157158

158159
/// Tracks whether a column holds plain numbers or parsed dates so we can
159160
/// format output appropriately (dates as RFC3339, spreads/shifts as days).
@@ -307,9 +308,16 @@ fn columns_from_cache(args: &Args, headers: &csv::ByteRecord) -> Option<Vec<(usi
307308
let file = std::fs::File::open(&cache_path).ok()?;
308309
let reader = std::io::BufReader::new(file);
309310

311+
let lines: Vec<String> = reader.lines().collect::<Result<_, _>>().ok()?;
312+
313+
// Validate that the cache has exactly as many records as the CSV has columns.
314+
// If it doesn't match (e.g. cache was generated with --select), ignore it.
315+
if lines.len() != headers.len() {
316+
return None;
317+
}
318+
310319
let mut result = Vec::new();
311-
for (i, line) in reader.lines().enumerate() {
312-
let curr_line = line.ok()?;
320+
for (i, curr_line) in lines.iter().enumerate() {
313321
let mut s_slice = curr_line.as_bytes().to_vec();
314322

315323
#[cfg(target_endian = "big")]
@@ -328,10 +336,7 @@ fn columns_from_cache(args: &Args, headers: &csv::ByteRecord) -> Option<Vec<(usi
328336
"DateTime" => ColType::DateTime,
329337
_ => continue,
330338
};
331-
// The JSONL line index `i` directly corresponds to column `i` in the CSV headers.
332-
if i < headers.len() {
333-
result.push((i, col_type));
334-
}
339+
result.push((i, col_type));
335340
}
336341

337342
if result.is_empty() {
@@ -662,7 +667,7 @@ fn fmt_timestamp(val: Option<f64>, ct: ColType) -> String {
662667
None => String::new(),
663668
Some(v) => {
664669
#[allow(clippy::cast_possible_truncation)]
665-
let ts = v as i64;
670+
let ts = v.round() as i64;
666671
match chrono::DateTime::from_timestamp_millis(ts) {
667672
None => String::new(),
668673
Some(dt) => {

0 commit comments

Comments
 (0)