Merge pull request #1399

Rename the attribute_extractor to meta_extractor

CHANGELOG.md

@@ -11,7 +11,11 @@ This changelog documents all notable user-facing changes of VAST.
 ### 🐞 Bug Fixes
 -->
 
-<!-- ## Unreleased -->
+## Unreleased
+
+- ⚡️ The previously deprecated `#timestamp` extractor has been removed from
+  the query language entirely.
+  [#1399](https://github.com/tenzir/vast/pull/1399)
 
 ## [2021.02.24]
 

libvast/src/concept/parseable/vast/expression.cpp

@@ -44,15 +44,6 @@ static predicate::operand to_field_extractor(std::vector<std::string> xs) {
   return field_extractor{std::move(field)};
 }
 
-static predicate::operand to_attr_extractor(std::string x) {
-  if (x == "timestamp") {
-    VAST_WARN("#timestamp queries are deprecated and should be replaced with "
-              ":timestamp");
-    return type_extractor{none_type{}.name("timestamp")};
-  }
-  return attribute_extractor{caf::atom_from_string(x)};
-}
-
 static predicate::operand to_type_extractor(type x) {
   return type_extractor{std::move(x)};
 }
@@ -134,15 +125,16 @@ static auto make_predicate_parser() {
   using parsers::chr;
   using namespace parser_literals;
   // clang-format off
-  auto id = +(alnum | chr{'_'} | chr{'-'});
+  // TODO: Align this with identifier_char.
   auto field_char = alnum | chr{'_'} | chr{'-'} | chr{':'};
   // A field cannot start with:
   //  - '-' to leave room for potential arithmetic expressions in operands
   //  - ':' so it won't be interpreted as a type extractor
   auto field = !(':'_p | '-') >> (+field_char % '.');
   auto operand
     = (parsers::data >> !(field_char | '.')) ->* to_data_operand
-    | '#' >> id ->* to_attr_extractor
+    | "#type"_p  ->* [] { return meta_extractor{meta_extractor::type}; }
+    | "#field"_p ->* [] { return meta_extractor{meta_extractor::field}; }
     | ':' >> parsers::type ->* to_type_extractor
     | field ->* to_field_extractor
     ;

libvast/src/expression.cpp

@@ -22,21 +22,17 @@
 
 namespace vast {
 
-// -- attribute_extractor ------------------------------------------------------
+// -- meta_extractor -----------------------------------------------------------
 
-attribute_extractor::attribute_extractor(caf::atom_value str) : attr{str} {
-  // nop
-}
-
-bool operator==(const attribute_extractor& x, const attribute_extractor& y) {
-  return x.attr == y.attr;
+bool operator==(const meta_extractor& x, const meta_extractor& y) {
+  return x.kind == y.kind;
 }
 
-bool operator<(const attribute_extractor& x, const attribute_extractor& y) {
-  return x.attr < y.attr;
+bool operator<(const meta_extractor& x, const meta_extractor& y) {
+  return x.kind < y.kind;
 }
 
-// -- field_extractor ------------------------------------------------------------
+// -- field_extractor ----------------------------------------------------------
 
 field_extractor::field_extractor(std::string f) : field{std::move(f)} {
 }

libvast/src/expression_visitors.cpp

@@ -254,21 +254,21 @@ caf::expected<void> validator::operator()(const predicate& p) {
 }
 
 caf::expected<void>
-validator::operator()(const attribute_extractor& ex, const data& d) {
-  if (ex.attr == atom::type_v
+validator::operator()(const meta_extractor& ex, const data& d) {
+  if (ex.kind == meta_extractor::type
       && !(caf::holds_alternative<std::string>(d)
            || caf::holds_alternative<pattern>(d)))
     return caf::make_error(ec::syntax_error,
-                           "type attribute extractor requires string or "
-                           "pattern operand",
-                           ex.attr, op_, d);
-  if (ex.attr == atom::field_v
+                           "type meta extractor requires string or pattern "
+                           "operand",
+                           "#type", op_, d);
+  if (ex.kind == meta_extractor::field
       && !(caf::holds_alternative<std::string>(d)
            || caf::holds_alternative<pattern>(d)))
     return caf::make_error(ec::syntax_error,
                            "field attribute extractor requires string or "
                            "pattern operand",
-                           ex.attr, op_, d);
+                           "#field", op_, d);
   return caf::no_error;
 }
 
@@ -349,7 +349,7 @@ caf::expected<expression> type_resolver::operator()(const predicate& p) {
 }
 
 caf::expected<expression>
-type_resolver::operator()(const attribute_extractor& ex, const data& d) {
+type_resolver::operator()(const meta_extractor& ex, const data& d) {
   // We're leaving all attributes alone, because both #type and #field operate
   // at a different granularity.
   return predicate{ex, op_, d};
@@ -458,8 +458,8 @@ bool matcher::operator()(const predicate& p) {
   return caf::visit(*this, p.lhs, p.rhs);
 }
 
-bool matcher::operator()(const attribute_extractor& e, const data& d) {
-  if (e.attr == atom::type_v) {
+bool matcher::operator()(const meta_extractor& e, const data& d) {
+  if (e.kind == meta_extractor::type) {
     VAST_ASSERT(caf::holds_alternative<std::string>(d));
     return evaluate(d, op_, type_.name());
   }

libvast/src/meta_index.cpp

@@ -164,8 +164,8 @@ std::vector<uuid> meta_index::lookup(const expression& expr) const {
         return result;
       };
       auto extract_expr = detail::overload{
-        [&](const attribute_extractor& lhs, const data& d) -> result_type {
-          if (lhs.attr == atom::type_v) {
+        [&](const meta_extractor& lhs, const data& d) -> result_type {
+          if (lhs.kind == meta_extractor::type) {
             // We don't have to look into the synopses for type queries, just
             // at the layout names.
             result_type result;
@@ -185,7 +185,7 @@ std::vector<uuid> meta_index::lookup(const expression& expr) const {
             // Re-establish potentially violated invariant.
             std::sort(result.begin(), result.end());
             return result;
-          } else if (lhs.attr == atom::field_v) {
+          } else if (lhs.kind == meta_extractor::field) {
             // We don't have to look into the synopses for type queries, just
             // at the layout names.
             result_type result;
@@ -217,7 +217,7 @@ std::vector<uuid> meta_index::lookup(const expression& expr) const {
             return result;
           }
           VAST_WARN("{} cannot process attribute extractor: {}",
-                    detail::pretty_type_name(this), lhs.attr);
+                    detail::pretty_type_name(this), lhs.kind);
           return all_partitions();
         },
         [&](const field_extractor& lhs, const data&) -> result_type {

libvast/src/system/partition.cpp

@@ -121,18 +121,18 @@ fetch_indexer(const PartitionState& state, const data_extractor& dx,
 /// @relates passive_partition_state
 template <typename PartitionState>
 indexer_actor
-fetch_indexer(const PartitionState& state, const attribute_extractor& ex,
+fetch_indexer(const PartitionState& state, const meta_extractor& ex,
               relational_operator op, const data& x) {
   VAST_TRACE_SCOPE("{} {} {}", VAST_ARG(ex), VAST_ARG(op), VAST_ARG(x));
   ids row_ids;
-  if (ex.attr == atom::type_v) {
+  if (ex.kind == meta_extractor::type) {
     // We know the answer immediately: all IDs that are part of the table.
     // However, we still have to "lift" this result into an actor for the
     // EVALUATOR.
     for (auto& [name, ids] : state.type_ids)
       if (evaluate(name, op, x))
         row_ids |= ids;
-  } else if (ex.attr == atom::field_v) {
+  } else if (ex.kind == meta_extractor::field) {
     auto s = caf::get_if<std::string>(&x);
     if (!s) {
       VAST_WARN("{} #field meta queries only support string "
@@ -161,7 +161,7 @@ fetch_indexer(const PartitionState& state, const attribute_extractor& ex,
       row_ids = partition_ids ^ row_ids;
     }
   } else {
-    VAST_WARN("{} got unsupported attribute: {}", state.self, ex.attr);
+    VAST_WARN("{} got unsupported attribute: {}", state.self, ex.kind);
     return {};
   }
   // TODO: Spawning a one-shot actor is quite expensive. Maybe the
@@ -194,7 +194,7 @@ evaluate(const PartitionState& state, const expression& expr) {
       return fetch_indexer(state, ext, pred.op, x);
     };
     auto v = detail::overload{
-      [&](const attribute_extractor& ex, const data& x) {
+      [&](const meta_extractor& ex, const data& x) {
         return get_indexer_handle(ex, x);
       },
       [&](const data_extractor& dx, const data& x) {

libvast/src/system/pivoter.cpp

@@ -129,7 +129,7 @@ caf::behavior pivoter(caf::stateful_actor<pivoter_state>* self, node_actor node,
         return;
       }
       auto expr
-        = conjunction{predicate{attribute_extractor{atom::type_v},
+        = conjunction{predicate{meta_extractor{meta_extractor::type},
                                 relational_operator::equal, data{st.target}},
                       predicate{field_extractor{pivot_field->name},
                                 relational_operator::in, data{xs}}};

libvast/src/system/sink_command.cpp

@@ -66,7 +66,7 @@ sink_command(const invocation& inv, caf::actor_system& sys, caf::actor snk) {
     auto expr = to<expression>(*query);
     if (!expr)
       return make_message(expr.error());
-    auto pred = predicate{attribute_extractor{atom::type_v},
+    auto pred = predicate{meta_extractor{meta_extractor::type},
                           relational_operator::equal, data{"pcap.packet"}};
     auto ast = conjunction{std::move(pred), std::move(*expr)};
     *query = to_string(ast);

libvast/src/table_slice.cpp

@@ -600,16 +600,16 @@ struct row_evaluator {
     return caf::visit(*this, p.lhs, p.rhs);
   }
 
-  bool operator()(const attribute_extractor& e, const data& d) {
+  bool operator()(const meta_extractor& e, const data& d) {
     // TODO: Transform this AST node into a constant-time lookup node (e.g.,
     // data_extractor). It's not necessary to iterate over the schema for
     // every row; this should happen upfront.
     auto&& layout = slice_.layout();
     // TODO: type and field queries don't produce false positives in the
     // partition. Is there actually any reason to do the check here?
-    if (e.attr == atom::type_v)
+    if (e.kind == meta_extractor::type)
       return evaluate(layout.name(), op_, d);
-    if (e.attr == atom::field_v) {
+    if (e.kind == meta_extractor::field) {
       auto s = caf::get_if<std::string>(&d);
       if (!s) {
         VAST_WARN("#field can only compare with string");

libvast/src/taxonomies.cpp

@@ -322,7 +322,7 @@ resolve_impl(const taxonomies& ts, const expression& e,
         auto make_meta_field_predicate
           = [&](relational_operator op, const vast::data&) {
               return [&, op](std::string item) {
-                return predicate{attribute_extractor{atom::field_v}, op,
+                return predicate{meta_extractor{meta_extractor::field}, op,
                                  vast::data{item}};
               };
             };

libvast/test/expression.cpp

@@ -54,7 +54,7 @@ struct fixture {
     // expr0 := !(x.y.z <= 42 && #foo == T)
     auto p0 = predicate{field_extractor{"x.y.z"},
                         relational_operator::less_equal, data{42}};
-    auto p1 = predicate{attribute_extractor{caf::atom("foo")},
+    auto p1 = predicate{meta_extractor{meta_extractor::field},
                         relational_operator::equal, data{true}};
     auto conj = conjunction{p0, p1};
     expr0 = negation{conj};
@@ -85,8 +85,7 @@ TEST(construction) {
   CHECK(get<data>(p0->rhs) == data{42});
   auto p1 = caf::get_if<predicate>(&c->at(1));
   REQUIRE(p1);
-  CHECK(attribute{caf::to_string(get<attribute_extractor>(p1->lhs).attr)}
-        == attribute{"foo"});
+  CHECK_EQUAL(get<meta_extractor>(p1->lhs).kind, meta_extractor::field);
   CHECK_EQUAL(p1->op, relational_operator::equal);
   CHECK(get<data>(p1->rhs) == data{true});
 }
@@ -135,7 +134,7 @@ TEST(normalization) {
   CHECK_EQUAL(normalize(*expr), *normalized);
   // The normalizer must not touch predicates with two extractors, regardless
   // of whether that's actually a valid construct.
-  expr = to<expression>("#foo == #bar");
+  expr = to<expression>(":foo == :bar");
   REQUIRE(expr);
   CHECK_EQUAL(normalize(*expr), *expr);
   MESSAGE("pushing down negations to predicate level");

libvast/test/expression_evaluation.cpp

@@ -54,20 +54,14 @@ struct fixture : fixtures::events {
 
 FIXTURE_SCOPE(evaluation_tests, fixture)
 
-TEST(evaluation - attribute extractor - #timestamp) {
-  auto expr = make_conn_expr("#timestamp <= 2009-11-18T08:09");
-  auto ids = evaluate(expr, zeek_conn_log_slice);
-  CHECK_EQUAL(ids, make_ids({{0, 5}}, zeek_conn_log_slice.rows()));
-}
-
-TEST(evaluation - attribute extractor - #type) {
+TEST(evaluation - meta extractor - #type) {
   auto expr = make_expr("#type == \"zeek.conn\"");
   auto ids = evaluate(expr, zeek_conn_log_slice);
   CHECK_EQUAL(ids, make_ids({{0, zeek_conn_log_slice.rows()}}));
 }
 
-TEST(evaluation - attribute extractor - #foo) {
-  auto expr = make_expr("#foo == 42");
+TEST(evaluation - meta extractor - #field) {
+  auto expr = make_expr("#field == \"a.b.c\"");
   auto ids = evaluate(expr, zeek_conn_log_slice);
   CHECK_EQUAL(ids.size(), zeek_conn_log_slice.rows());
   CHECK(all<0>(ids));

libvast/test/expression_parseable.cpp

@@ -14,6 +14,8 @@
 #include "vast/expression.hpp"
 #define SUITE expression
 
+#include "vast/fwd.hpp"
+
 #include "vast/test/test.hpp"
 
 #include "vast/concept/parseable/to.hpp"
@@ -22,14 +24,13 @@
 #include "vast/concept/printable/stream.hpp"
 #include "vast/concept/printable/to_string.hpp"
 #include "vast/concept/printable/vast/expression.hpp"
-#include "vast/fwd.hpp"
 
 #include <caf/sum_type.hpp>
 
 using namespace vast;
 using namespace std::string_literals;
 
-TEST(parseable/printable - predicate) {
+TEST(parseable / printable - predicate) {
   predicate pred;
   // LHS: schema, RHS: data
   MESSAGE("x.y.z == 42");
@@ -65,10 +66,10 @@ TEST(parseable/printable - predicate) {
   MESSAGE("#type != \"foo\"");
   str = "#type != \"foo\"";
   CHECK(parsers::predicate(str, pred));
-  CHECK(caf::holds_alternative<attribute_extractor>(pred.lhs));
+  CHECK(caf::holds_alternative<meta_extractor>(pred.lhs));
   CHECK(caf::holds_alternative<data>(pred.rhs));
-  CHECK(caf::get<attribute_extractor>(pred.lhs)
-        == attribute_extractor{atom::type_v});
+  CHECK(caf::get<meta_extractor>(pred.lhs)
+        == meta_extractor{meta_extractor::type});
   CHECK(pred.op == relational_operator::not_equal);
   CHECK(caf::get<data>(pred.rhs) == data{"foo"});
   CHECK_EQUAL(to_string(pred), str);
@@ -92,15 +93,15 @@ TEST(parseable/printable - predicate) {
   CHECK(pred.op == relational_operator::greater_equal);
   CHECK(caf::get<data>(pred.rhs) == data{-4.8});
   CHECK_EQUAL(to_string(pred), str);
-  // LHS: data, RHS: time
-  MESSAGE("now > #timestamp");
-  str = "now > #timestamp";
+  // LHS: data, RHS: typename
+  MESSAGE("\"zeek.\" in #type");
+  str = "\"zeek.\" in #type";
   CHECK(parsers::predicate(str, pred));
   CHECK(caf::holds_alternative<data>(pred.lhs));
-  CHECK(caf::holds_alternative<type_extractor>(pred.rhs));
-  CHECK(pred.op == relational_operator::greater);
-  CHECK(caf::get<type_extractor>(pred.rhs)
-        == type_extractor{none_type{}.name("timestamp")});
+  CHECK(caf::holds_alternative<meta_extractor>(pred.rhs));
+  CHECK(pred.op == relational_operator::in);
+  CHECK(caf::get<meta_extractor>(pred.rhs)
+        == meta_extractor{meta_extractor::type});
   // LHS: schema, RHS: schema
   MESSAGE("x.a_b == y.c_d");
   str = "x.a_b == y.c_d";
@@ -132,7 +133,7 @@ TEST(parseable - expression) {
   CHECK_EQUAL(expr, expression(conjunction{p1, negation{p2}, p1}));
   CHECK(parsers::expr("x > 0 && x < 42 && a.b == x.y"s, expr));
   CHECK(parsers::expr(
-    "#timestamp > 2018-07-04+12:00:00.0 && #timestamp < 2018-07-04+23:55:04.0"s,
+    ":timestamp > 2018-07-04+12:00:00.0 && :timestamp < 2018-07-04+23:55:04.0"s,
     expr));
   auto x = caf::get_if<conjunction>(&expr);
   REQUIRE(x);

libvast/test/flatbuffers.cpp

@@ -250,10 +250,10 @@ TEST(full partition roundtrip) {
     vast::predicate{vast::field_extractor{".x"},
                     vast::relational_operator::equal, vast::data{1u}}};
   auto type_equals_y = vast::expression{
-    vast::predicate{vast::attribute_extractor{vast::atom::type_v},
+    vast::predicate{vast::meta_extractor{vast::meta_extractor::type},
                     vast::relational_operator::equal, vast::data{"y"}}};
   auto type_equals_foo = vast::expression{
-    vast::predicate{vast::attribute_extractor{vast::atom::type_v},
+    vast::predicate{vast::meta_extractor{vast::meta_extractor::type},
                     vast::relational_operator::equal, vast::data{"foo"}}};
   // For the query `x == 0`, we expect one result.
   test_expression(x_equals_zero, 1);