Skip to content

Commit

Permalink
Fix incorrect timeseries fields in error message
Browse files Browse the repository at this point in the history
- Fixes #5832
- Uses timeseries fields in error message rather than filter identifiers
  • Loading branch information
bnaecker committed May 29, 2024
1 parent 1fe55e9 commit fc8b7a3
Showing 1 changed file with 31 additions and 2 deletions.
33 changes: 31 additions & 2 deletions oximeter/db/src/oxql/ast/table_ops/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,11 @@ impl Filter {
// You give nothing, you get nothing.
return Ok(tables.to_vec());
};
let ident_names = self.ident_names();

// There are extra, implied names that depend on the data type of the
// timeseries itself, check those as well.
let extras = implicit_field_names(first_timeseries);
let ident_names = self.ident_names();
let not_valid = ident_names
.iter()
.filter(|&&name| {
Expand All @@ -316,7 +316,13 @@ impl Filter {
valid for its input timeseries. Invalid identifiers: {:?}, \
timeseries fields: {:?}",
not_valid,
ident_names.union(&extras),
first_timeseries
.fields
.keys()
.map(String::as_str)
.collect::<BTreeSet<_>>()
.union(&extras)
.collect::<BTreeSet<_>>(),
);

// Filter each input table in succession.
Expand Down Expand Up @@ -1353,4 +1359,27 @@ mod tests {
"It's not an error to filter an empty table"
);
}

#[test]
fn test_error_message_with_invalid_field_names() {
let ts = Timeseries::new(
std::iter::once((String::from("foo"), FieldValue::U8(0))),
DataType::Double,
MetricType::Gauge,
)
.unwrap();
let table = Table::from_timeseries("foo", std::iter::once(ts)).unwrap();
let filt = query_parser::filter_expr("bar == 0").unwrap();
let msg = filt
.apply(&[table])
.expect_err(
"Applying a filter with a name that doesn't appear \
in the timeseries should be an error",
)
.to_string();
println!("{msg}");
assert!(
msg.contains(r#"timeseries fields: {"datum", "foo", "timestamp"}"#)
);
}
}

0 comments on commit fc8b7a3

Please sign in to comment.