From a5b906eea3f380fbd838bd6ee6599c510317a3e0 Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Sat, 25 Oct 2014 22:45:32 +0200 Subject: [PATCH] frontend/grammar: Access fields by their name rather than '.'. There's no more need for a special and somewhat weird syntax. --- src/frontend/grammar.y | 42 +++---------- t/integration/filter.sh | 8 +-- t/unit/frontend/parser_test.c | 112 +++++++++++++++++----------------- 3 files changed, 68 insertions(+), 94 deletions(-) diff --git a/src/frontend/grammar.y b/src/frontend/grammar.y index 92b55eb..fed30d2 100644 --- a/src/frontend/grammar.y +++ b/src/frontend/grammar.y @@ -47,9 +47,6 @@ * private helper functions */ -static sdb_store_matcher_t * -name_matcher(const char *type_name, const char *cmp, sdb_store_expr_t *expr); - static sdb_store_matcher_t * name_iter_matcher(int m_type, const char *type_name, const char *cmp, sdb_store_expr_t *expr); @@ -458,13 +455,6 @@ compare_matcher: sdb_object_deref(SDB_OBJ($3)); } | - IDENTIFIER cmp expression - { - $$ = name_matcher($1, $2, $3); - free($1); $1 = NULL; - sdb_object_deref(SDB_OBJ($3)); - } - | ANY IDENTIFIER cmp expression { $$ = name_iter_matcher(MATCHER_ANY, $2, $3, $4); @@ -547,10 +537,16 @@ expression: sdb_object_deref(SDB_OBJ($3)); $3 = NULL; } | - '.' IDENTIFIER + IDENTIFIER { - int field = sdb_store_parse_field_name($2); - free($2); $2 = NULL; + int field; + /* TODO: this only works as long as queries + * are limited to hosts */ + if (!strcasecmp($1, "host")) + field = SDB_FIELD_NAME; + else + field = sdb_store_parse_field_name($1); + free($1); $1 = NULL; $$ = sdb_store_expr_fieldvalue(field); } | @@ -659,26 +655,6 @@ sdb_fe_yyerror(YYLTYPE *lval, sdb_fe_yyscan_t scanner, const char *msg) sdb_log(SDB_LOG_ERR, "frontend: parse error: %s", msg); } /* sdb_fe_yyerror */ -static sdb_store_matcher_t * -name_matcher(const char *type_name, const char *cmp, sdb_store_expr_t *expr) -{ - int type = sdb_store_parse_object_type(type_name); - sdb_store_matcher_op_cb cb = sdb_store_parse_matcher_op(cmp); - sdb_store_expr_t *e; - sdb_store_matcher_t *m; - assert(cb); - - /* TODO: this only works as long as queries - * are limited to hosts */ - if (type != SDB_HOST) - return NULL; - - e = sdb_store_expr_fieldvalue(SDB_FIELD_NAME); - m = cb(e, expr); - sdb_object_deref(SDB_OBJ(e)); - return m; -} /* name_matcher */ - static sdb_store_matcher_t * name_iter_matcher(int m_type, const char *type_name, const char *cmp, sdb_store_expr_t *expr) diff --git a/t/integration/filter.sh b/t/integration/filter.sh index b4930b4..da75b9a 100755 --- a/t/integration/filter.sh +++ b/t/integration/filter.sh @@ -50,7 +50,7 @@ sleep 3 output="$( run_sysdb -H "$SOCKET_FILE" \ -c "LOOKUP hosts MATCHING ANY attribute != 'architecture' - FILTER .age >= 0s" )" + FILTER age >= 0s" )" echo "$output" \ | grep -F '"localhost"' echo "$output" | grep -F 'some.host.name' && exit 1 @@ -60,11 +60,11 @@ echo "$output" | grep -F 'some.host.name' && exit 1 output="$( run_sysdb -H "$SOCKET_FILE" \ -c "LOOKUP hosts MATCHING ANY attribute != 'architecture' - FILTER .last_update < 2Y" )" + FILTER last_update < 2Y" )" echo $output | grep -E '^\[\]$' output="$( run_sysdb -H "$SOCKET_FILE" \ - -c "LOOKUP hosts FILTER 'backend::mock_plugin' IN .backend" )" + -c "LOOKUP hosts FILTER 'backend::mock_plugin' IN backend" )" echo "$output" \ | grep -F '"host1.example.com"' \ | grep -F '"host2.example.com"' \ @@ -72,7 +72,7 @@ echo "$output" \ | grep -F '"other.host.name"' \ | grep -F '"some.host.name"' output="$( run_sysdb -H "$SOCKET_FILE" \ - -c "LOOKUP hosts FILTER 'invalid' IN .backend" )" + -c "LOOKUP hosts FILTER 'invalid' IN backend" )" echo $output | grep -E '^\[\]$' stop_sysdbd diff --git a/t/unit/frontend/parser_test.c b/t/unit/frontend/parser_test.c index fd9e32f..6008f7c 100644 --- a/t/unit/frontend/parser_test.c +++ b/t/unit/frontend/parser_test.c @@ -91,18 +91,18 @@ START_TEST(test_parse) "ANY service =~ 'r'", -1, 1, CONNECTION_LOOKUP }, { "LOOKUP hosts MATCHING " "host =~ 'p' " - "FILTER .age > 1D", -1, 1, CONNECTION_LOOKUP }, + "FILTER age > 1D", -1, 1, CONNECTION_LOOKUP }, { "LOOKUP hosts MATCHING " "host =~ 'p' " - "FILTER .age > 1D AND " - ".interval < 240s" , -1, 1, CONNECTION_LOOKUP }, + "FILTER age > 1D AND " + "interval < 240s" , -1, 1, CONNECTION_LOOKUP }, { "LOOKUP hosts MATCHING " "host =~ 'p' " - "FILTER NOT .age>1D", -1, 1, CONNECTION_LOOKUP }, + "FILTER NOT age>1D", -1, 1, CONNECTION_LOOKUP }, { "LOOKUP hosts MATCHING " "host =~ 'p' " - "FILTER .age>" - ".interval", -1, 1, CONNECTION_LOOKUP }, + "FILTER age>" + "interval", -1, 1, CONNECTION_LOOKUP }, { "TIMESERIES 'host'.'metric' " "START 2014-01-01 " @@ -186,8 +186,6 @@ START_TEST(test_parse) { "LOOKUP hosts MATCHING " "NOT attribute['foo'] " "IS NULL", -1, 1, CONNECTION_LOOKUP }, - { "LOOKUP hosts MATCHING " - "host IS NULL", -1, -1, 0 }, { "LOOKUP hosts MATCHING " "ANY service IS NULL", -1, -1, 0 }, @@ -379,36 +377,36 @@ START_TEST(test_parse_matcher) { "attribute['foo'] IS NOT NULL", -1, MATCHER_ISNNULL }, /* object field matchers */ - { ".name < 'a'", -1, MATCHER_LT }, - { ".name <= 'a'", -1, MATCHER_LE }, - { ".name = 'a'", -1, MATCHER_EQ }, - { ".name != 'a'", -1, MATCHER_NE }, - { ".name >= 'a'", -1, MATCHER_GE }, - { ".name > 'a'", -1, MATCHER_GT }, - { ".last_update < 2014-10-01", -1, MATCHER_LT }, - { ".last_update <= 2014-10-01", -1, MATCHER_LE }, - { ".last_update = 2014-10-01", -1, MATCHER_EQ }, - { ".last_update != 2014-10-01", -1, MATCHER_NE }, - { ".last_update >= 2014-10-01", -1, MATCHER_GE }, - { ".last_update > 2014-10-01", -1, MATCHER_GT }, - { ".Last_Update >= 24D", -1, MATCHER_GE }, - { ".age < 20s", -1, MATCHER_LT }, - { ".age <= 20s", -1, MATCHER_LE }, - { ".age = 20s", -1, MATCHER_EQ }, - { ".age != 20s", -1, MATCHER_NE }, - { ".age >= 20s", -1, MATCHER_GE }, - { ".age > 20s", -1, MATCHER_GT }, - { ".AGE <= 1m", -1, MATCHER_LE }, - { ".age > 1M", -1, MATCHER_GT }, - { ".age != 20Y", -1, MATCHER_NE }, - { ".age <= 2 * .interval", -1, MATCHER_LE }, - { ".interval < 20s", -1, MATCHER_LT }, - { ".interval <= 20s", -1, MATCHER_LE }, - { ".interval = 20s", -1, MATCHER_EQ }, - { ".interval != 20s", -1, MATCHER_NE }, - { ".interval >= 20s", -1, MATCHER_GE }, - { ".interval > 20s", -1, MATCHER_GT }, - { "'be' IN .backend", -1, MATCHER_IN }, + { "name < 'a'", -1, MATCHER_LT }, + { "name <= 'a'", -1, MATCHER_LE }, + { "name = 'a'", -1, MATCHER_EQ }, + { "name != 'a'", -1, MATCHER_NE }, + { "name >= 'a'", -1, MATCHER_GE }, + { "name > 'a'", -1, MATCHER_GT }, + { "last_update < 2014-10-01", -1, MATCHER_LT }, + { "last_update <= 2014-10-01", -1, MATCHER_LE }, + { "last_update = 2014-10-01", -1, MATCHER_EQ }, + { "last_update != 2014-10-01", -1, MATCHER_NE }, + { "last_update >= 2014-10-01", -1, MATCHER_GE }, + { "last_update > 2014-10-01", -1, MATCHER_GT }, + { "Last_Update >= 24D", -1, MATCHER_GE }, + { "age < 20s", -1, MATCHER_LT }, + { "age <= 20s", -1, MATCHER_LE }, + { "age = 20s", -1, MATCHER_EQ }, + { "age != 20s", -1, MATCHER_NE }, + { "age >= 20s", -1, MATCHER_GE }, + { "age > 20s", -1, MATCHER_GT }, + { "AGE <= 1m", -1, MATCHER_LE }, + { "age > 1M", -1, MATCHER_GT }, + { "age != 20Y", -1, MATCHER_NE }, + { "age <= 2 * interval", -1, MATCHER_LE }, + { "interval < 20s", -1, MATCHER_LT }, + { "interval <= 20s", -1, MATCHER_LE }, + { "interval = 20s", -1, MATCHER_EQ }, + { "interval != 20s", -1, MATCHER_NE }, + { "interval >= 20s", -1, MATCHER_GE }, + { "interval > 20s", -1, MATCHER_GT }, + { "'be' IN backend", -1, MATCHER_IN }, /* check operator precedence */ { "host = 'name' OR " @@ -437,7 +435,7 @@ START_TEST(test_parse_matcher) /* syntax errors */ { "LIST", -1, -1 }, { "foo &^ bar", -1, -1 }, - { ".invalid", -1, -1 }, + { "invalid", -1, -1 }, }; size_t i; @@ -498,35 +496,35 @@ START_TEST(test_parse_expr) { "5 % 2", -1, 0 }, /* queryable fields */ - { ".last_update", -1, FIELD_VALUE }, - { ".AGE", -1, FIELD_VALUE }, - { ".interval", -1, FIELD_VALUE }, - { ".Last_Update", -1, FIELD_VALUE }, - { ".backend", -1, FIELD_VALUE }, + { "last_update", -1, FIELD_VALUE }, + { "AGE", -1, FIELD_VALUE }, + { "interval", -1, FIELD_VALUE }, + { "Last_Update", -1, FIELD_VALUE }, + { "backend", -1, FIELD_VALUE }, /* attributes */ { "attribute['foo']", -1, ATTR_VALUE }, /* arithmetic expressions */ - { ".age + .age", -1, SDB_DATA_ADD }, - { ".age - .age", -1, SDB_DATA_SUB }, - { ".age * .age", -1, SDB_DATA_MUL }, - { ".age / .age", -1, SDB_DATA_DIV }, - { ".age % .age", -1, SDB_DATA_MOD }, - { ".age || .age", -1, SDB_DATA_CONCAT }, + { "age + age", -1, SDB_DATA_ADD }, + { "age - age", -1, SDB_DATA_SUB }, + { "age * age", -1, SDB_DATA_MUL }, + { "age / age", -1, SDB_DATA_DIV }, + { "age % age", -1, SDB_DATA_MOD }, + { "age || age", -1, SDB_DATA_CONCAT }, /* operator precedence */ - { ".age + .age * .age", -1, SDB_DATA_ADD }, - { ".age * .age + .age", -1, SDB_DATA_ADD }, - { ".age + .age - .age", -1, SDB_DATA_SUB }, - { ".age - .age + .age", -1, SDB_DATA_ADD }, - { "(.age + .age) * .age", -1, SDB_DATA_MUL }, - { ".age + (.age * .age)", -1, SDB_DATA_ADD }, + { "age + age * age", -1, SDB_DATA_ADD }, + { "age * age + age", -1, SDB_DATA_ADD }, + { "age + age - age", -1, SDB_DATA_SUB }, + { "age - age + age", -1, SDB_DATA_ADD }, + { "(age + age) * age", -1, SDB_DATA_MUL }, + { "age + (age * age)", -1, SDB_DATA_ADD }, /* syntax errors */ { "LIST", -1, INT_MAX }, { "foo &^ bar", -1, INT_MAX }, - { ".invalid", -1, INT_MAX }, + { "invalid", -1, INT_MAX }, }; size_t i; -- 2.30.2