Skip to content

Commit 607a9c7

Browse files
authored
Prevent detect columns from creating invalid records with duplicate keys (nushell#16527)
## References Fixes nushell#11791 ## Release notes summary - What our users need to know Previously `detect columns` created records (rows) with duplicate key names under some circumstances. The resulting table behaved inconsistently with different commands. Example: ```nushell let data = "meooooow cat\nkitty kitty woof" $data | detect columns # => ╭───┬──────────┬───────╮ # => │ # │ meooooow │ cat │ # => ├───┼──────────┼───────┤ # => │ 0 │ kitty │ kitty │ # => ╰───┴──────────┴───────╯ $data | detect columns | get 0.cat # => woof ``` Now it will result in an error suggesting using `detect columns --guess` or `parse` ## Tasks after submitting <!-- Remove any tasks which aren't relevant for your PR, or add your own --> - [ ] Update the [documentation](https://github.com/nushell/nushell.github.io)
1 parent e15e3ec commit 607a9c7

File tree

3 files changed

+33
-2
lines changed

3 files changed

+33
-2
lines changed

crates/nu-command/src/strings/detect_columns.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ fn detect_columns(
237237
args: Arguments,
238238
) -> Result<PipelineData, ShellError> {
239239
let name_span = call.head;
240+
let input_span = input.span().unwrap_or(Span::unknown());
240241
let input = input.collect_string("", &args.config)?;
241242

242243
let input: Vec<_> = input
@@ -311,11 +312,20 @@ fn detect_columns(
311312
}
312313
}
313314

314-
match &args.range {
315+
let has_column_duplicates = record.columns().duplicates().count() > 0;
316+
if has_column_duplicates {
317+
return Err(ShellError::ColumnDetectionFailure {
318+
bad_value: input_span,
319+
failure_site: name_span,
320+
});
321+
}
322+
323+
Ok(match &args.range {
315324
Some(range) => merge_record(record, range, name_span),
316325
None => Value::record(record, name_span),
317-
}
326+
})
318327
})
328+
.collect::<Result<Vec<_>, _>>()?
319329
.into_pipeline_data(call.head, engine_state.signals().clone()))
320330
} else {
321331
Ok(PipelineData::empty())

crates/nu-command/tests/commands/detect_columns.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,10 @@ drwxr-xr-x 4 root root 4.0K Mar 20 08:18 ~(char nl)
9494
assert_eq!(out.out, "true");
9595
})
9696
}
97+
98+
#[test]
99+
fn detect_columns_may_fail() {
100+
let out =
101+
nu!(r#""meooooow cat\nkitty kitty woof" | try { detect columns } catch { "failed" }"#);
102+
assert_eq!(out.out, "failed");
103+
}

crates/nu-protocol/src/errors/shell_error/mod.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,20 @@ pub enum ShellError {
667667
creation_site: Span,
668668
},
669669

670+
/// Failed to detect columns
671+
///
672+
/// ## Resolution
673+
///
674+
/// Use `detect columns --guess` or `parse` instead
675+
#[error("Failed to detect columns")]
676+
#[diagnostic(code(nu::shell::failed_to_detect_columns))]
677+
ColumnDetectionFailure {
678+
#[label = "value coming from here"]
679+
bad_value: Span,
680+
#[label = "tried to detect columns here"]
681+
failure_site: Span,
682+
},
683+
670684
/// Attempted to us a relative range on an infinite stream
671685
///
672686
/// ## Resolution

0 commit comments

Comments
 (0)