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.

diff --git a/src/frontend/grammar.y b/src/frontend/grammar.y
index 23ba07ffaed4a72425173ab890d9eb3d4e690d28..376c84afdb06bac4965e16c27b2bbb90662fd0ee 100644 (file)
--- a/src/frontend/grammar.y
+++ b/src/frontend/grammar.y
@@ -38,6 +38,8 @@
 #include "utils/error.h"
 #include "utils/llist.h"
 
+#include <assert.h>
+
 #include <stdio.h>
 #include <string.h>
 
@@ -431,17 +433,14 @@ matcher:
                }
        ;
 
-/*
- * <object_type>.<object_attr> <cmp> <value>
- *
- * 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 1739296ea1b4222cb9e4617c3e31df29fe5d4fa4..f9ffabe6d08d39632bb6823cc728af8eb9b6f465 100644 (file)
--- 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 075a4bc0e475791fba0cf8ace1ebc7de60eb206e..bcf00b22088ca27e3a3c3f1906cd660b665a91f5 100644 (file)
--- 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 */