From 241a7b9ab622e84960471bc9c70fbedccbbdff2d Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Tue, 21 Oct 2014 09:13:47 +0200 Subject: [PATCH] frontend/grammar: Unified field and attribute value matchers. Use generic expressions and the new compare matchers instead. As a side-effect of this change, not-operators (!=, etc.) no longer match on NULL values (attribute does not exist). This is intended and should have been like this in the first place (we've got 'IS NULL' for that purpose). For example, `attribute['foo'] != 123' did previously match if attribute 'foo' did not exist but now this is no longer the case. --- src/frontend/grammar.y | 25 +++++++--------------- t/unit/core/store_lookup_test.c | 35 +++++++++++++++--------------- t/unit/frontend/parser_test.c | 38 ++++++++++++++++----------------- 3 files changed, 45 insertions(+), 53 deletions(-) diff --git a/src/frontend/grammar.y b/src/frontend/grammar.y index 23ba07f..376c84a 100644 --- a/src/frontend/grammar.y +++ b/src/frontend/grammar.y @@ -38,6 +38,8 @@ #include "utils/error.h" #include "utils/llist.h" +#include + #include #include @@ -431,17 +433,14 @@ matcher: } ; -/* - * . - * - * Parse matchers comparing object attributes with a value. - */ compare_matcher: - '.' IDENTIFIER cmp expression + expression cmp expression { - $$ = sdb_store_matcher_parse_field_cmp($2, $3, $4); - free($2); $2 = NULL; - sdb_object_deref(SDB_OBJ($4)); + sdb_store_matcher_op_cb cb = sdb_store_parse_matcher_op($2); + assert(cb); /* else, the grammar accepts invalid 'cmp' */ + $$ = cb($1, $3); + sdb_object_deref(SDB_OBJ($1)); + sdb_object_deref(SDB_OBJ($3)); } | IDENTIFIER cmp expression @@ -451,14 +450,6 @@ compare_matcher: sdb_object_deref(SDB_OBJ($3)); } | - IDENTIFIER '[' STRING ']' cmp expression - { - $$ = sdb_store_matcher_parse_cmp($1, $3, $5, $6); - free($1); $1 = NULL; - free($3); $3 = NULL; - sdb_object_deref(SDB_OBJ($6)); - } - | expression IS NULL_T { $$ = sdb_store_isnull_matcher($1); diff --git a/t/unit/core/store_lookup_test.c b/t/unit/core/store_lookup_test.c index 1739296..f9ffabe 100644 --- a/t/unit/core/store_lookup_test.c +++ b/t/unit/core/store_lookup_test.c @@ -64,6 +64,7 @@ populate(void) } attrs[] = { { "a", "k1", { SDB_TYPE_STRING, { .string = "v1" } } }, { "a", "k2", { SDB_TYPE_INTEGER, { .integer = 123 } } }, + { "b", "k1", { SDB_TYPE_STRING, { .string = "v2" } } }, }; size_t i; @@ -820,46 +821,46 @@ START_TEST(test_scan) "OBJ\\[service\\]\\{ NAME\\{ NULL, "PTR_RE" } \\}" }, { "service !~ 's'", NULL, 1, "\\(NOT, OBJ\\[service\\]\\{ NAME\\{ NULL, "PTR_RE" } \\}\\)" }, - { "attribute = 'k1'", NULL, 1, + { "attribute = 'k1'", NULL, 2, "OBJ\\[attribute\\]\\{ NAME\\{ 'k1', \\(nil\\) \\} " }, { "attribute = 'k1'", "host = 'x'", 0, /* filter never matches */ "OBJ\\[attribute\\]\\{ NAME\\{ 'k1', \\(nil\\) \\} " }, { "attribute = 'k1'", - "NOT attribute['x'] = ''", 1, /* filter always matches */ + "NOT attribute['x'] = ''", 2, /* filter always matches */ "OBJ\\[attribute\\]\\{ NAME\\{ 'k1', \\(nil\\) \\} " }, { "attribute = 'x'", NULL, 0, "OBJ\\[attribute\\]\\{ NAME\\{ 'x', \\(nil\\) \\}" }, { "attribute['k1'] = 'v1'", NULL, 1, - "ATTR\\[k1\\]\\{ VALUE\\{ 'v1', \\(nil\\) \\} \\}" }, - { "attribute['k1'] IS NULL", NULL, 2, + "CMP_MATCHER\\(15\\)" }, + { "attribute['k1'] IS NULL", NULL, 1, "\\(IS NULL\\)" }, { "attribute['x1'] IS NULL", NULL, 3, "\\(IS NULL\\)" }, - { "attribute['k1'] IS NOT NULL", NULL, 1, + { "attribute['k1'] IS NOT NULL", NULL, 2, "\\(IS NOT NULL\\)" }, { "attribute['x1'] IS NOT NULL", NULL, 0, "\\(IS NOT NULL\\)" }, { "attribute['k2'] < 123", NULL, 0, - "ATTR\\[k2\\]\\{ < 123 \\}" }, + "CMP_MATCHER\\(13\\)" }, { "attribute['k2'] <= 123", NULL, 1, - "ATTR\\[k2\\]\\{ <= 123 \\}" }, + "CMP_MATCHER\\(14\\)" }, { "attribute['k2'] >= 123", NULL, 1, - "ATTR\\[k2\\]\\{ >= 123 \\}" }, + "CMP_MATCHER\\(17\\)" }, { "attribute['k2'] > 123", NULL, 0, - "ATTR\\[k2\\]\\{ > 123 \\}" }, + "CMP_MATCHER\\(18\\)" }, { "attribute['k2'] = 123", NULL, 1, - "ATTR\\[k2\\]\\{ = 123 \\}" }, - { "attribute['k2'] != 123", NULL, 2, - "\\(NOT, ATTR\\[k2\\]\\{ = 123 \\}\\)" }, - { "attribute['k1'] != 'v1'", NULL, 2, - "\\(NOT, ATTR\\[k1\\]\\{ VALUE\\{ 'v1', \\(nil\\) \\} \\}\\)" }, - { "attribute['k1'] != 'v2'", NULL, 3, - "\\(NOT, ATTR\\[k1\\]\\{ VALUE\\{ 'v2', \\(nil\\) \\} \\}\\)" }, + "CMP_MATCHER\\(15\\)" }, + { "attribute['k2'] != 123", NULL, 0, + "CMP_MATCHER\\(16\\)" }, + { "attribute['k1'] != 'v1'", NULL, 1, + "CMP_MATCHER\\(16\\)" }, + { "attribute['k1'] != 'v2'", NULL, 1, + "CMP_MATCHER\\(16\\)" }, { "attribute != 'x' " "AND attribute['y'] !~ 'x'", NULL, 3, "\\(AND, " "\\(NOT, OBJ\\[attribute\\]\\{ NAME\\{ 'x', \\(nil\\) \\} \\}\\), " - "\\(NOT, ATTR\\[y\\]\\{ VALUE\\{ NULL, "PTR_RE" \\} \\}\\)\\)" }, + "CMP_MATCHER\\(21\\)" }, }; int check, n; diff --git a/t/unit/frontend/parser_test.c b/t/unit/frontend/parser_test.c index 075a4bc..bcf00b2 100644 --- a/t/unit/frontend/parser_test.c +++ b/t/unit/frontend/parser_test.c @@ -329,39 +329,39 @@ START_TEST(test_parse_matcher) "service =~ 'pattern'", -1, MATCHER_OR }, { "NOT host = 'host'", -1, MATCHER_NOT }, /* numeric expressions */ - { "attribute['foo'] < 123", -1, MATCHER_LT }, - { "attribute['foo'] <= 123", -1, MATCHER_LE }, - { "attribute['foo'] = 123", -1, MATCHER_EQ }, - { "attribute['foo'] >= 123", -1, MATCHER_GE }, - { "attribute['foo'] > 123", -1, MATCHER_GT }, + { "attribute['foo'] < 123", -1, MATCHER_CMP_LT }, + { "attribute['foo'] <= 123", -1, MATCHER_CMP_LE }, + { "attribute['foo'] = 123", -1, MATCHER_CMP_EQ }, + { "attribute['foo'] >= 123", -1, MATCHER_CMP_GE }, + { "attribute['foo'] > 123", -1, MATCHER_CMP_GT }, /* datetime expressions */ { "attribute['foo'] = " - "2014-08-16", -1, MATCHER_EQ }, + "2014-08-16", -1, MATCHER_CMP_EQ }, { "attribute['foo'] = " - "17:23", -1, MATCHER_EQ }, + "17:23", -1, MATCHER_CMP_EQ }, { "attribute['foo'] = " - "17:23:53", -1, MATCHER_EQ }, + "17:23:53", -1, MATCHER_CMP_EQ }, { "attribute['foo'] = " - "17:23:53.123", -1, MATCHER_EQ }, + "17:23:53.123", -1, MATCHER_CMP_EQ }, { "attribute['foo'] = " - "17:23:53.123456789", -1, MATCHER_EQ }, + "17:23:53.123456789", -1, MATCHER_CMP_EQ }, { "attribute['foo'] = " - "2014-08-16 17:23", -1, MATCHER_EQ }, + "2014-08-16 17:23", -1, MATCHER_CMP_EQ }, { "attribute['foo'] = " - "2014-08-16 17:23:53", -1, MATCHER_EQ }, + "2014-08-16 17:23:53", -1, MATCHER_CMP_EQ }, /* NULL; while this is an implementation detail, * IS NULL currently maps to an equality matcher */ { "attribute['foo'] IS NULL", -1, MATCHER_ISNULL }, { "attribute['foo'] IS NOT NULL", -1, MATCHER_ISNNULL }, /* object field matchers */ - { ".last_update < 10s", -1, MATCHER_LT }, - { ".AGE <= 1m", -1, MATCHER_LE }, - { ".interval = 10h", -1, MATCHER_EQ }, - { ".Last_Update >= 24D", -1, MATCHER_GE }, - { ".age > 1M", -1, MATCHER_GT }, - { ".age != 20Y", -1, MATCHER_NOT }, - { ".age <= 2 * .interval", -1, MATCHER_LE }, + { ".last_update < 10s", -1, MATCHER_CMP_LT }, + { ".AGE <= 1m", -1, MATCHER_CMP_LE }, + { ".interval = 10h", -1, MATCHER_CMP_EQ }, + { ".Last_Update >= 24D", -1, MATCHER_CMP_GE }, + { ".age > 1M", -1, MATCHER_CMP_GT }, + { ".age != 20Y", -1, MATCHER_CMP_NE }, + { ".age <= 2 * .interval", -1, MATCHER_CMP_LE }, { "'be' IN .backend", -1, MATCHER_IN }, /* check operator precedence */ -- 2.30.2