From 489939f4470a3f89e77cf18f1426f42685627fca Mon Sep 17 00:00:00 2001
From: Benjamin Naecker <ben@oxide.computer>
Date: Wed, 29 May 2024 21:15:13 +0000
Subject: [PATCH] Fix incorrect timeseries fields in error message

- Fixes #5832
- Uses timeseries fields in error message rather than filter identifiers
---
 oximeter/db/src/oxql/ast/table_ops/filter.rs | 30 ++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/oximeter/db/src/oxql/ast/table_ops/filter.rs b/oximeter/db/src/oxql/ast/table_ops/filter.rs
index e5963fe69c5..e6a061b9527 100644
--- a/oximeter/db/src/oxql/ast/table_ops/filter.rs
+++ b/oximeter/db/src/oxql/ast/table_ops/filter.rs
@@ -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| {
@@ -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.
@@ -1353,4 +1359,24 @@ 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"}"#));
+    }
 }