analyzer: Analyze expressions as well; validate typed expressions.

Also, added various tests for typed expressions.

diff --git a/src/core/store-private.h b/src/core/store-private.h
index 3ae6c437d20f6b63a4376e610ac086bc576fd5f5..97e8a6c2fcce7fff076b2f0275ead0a2ae42306a 100644 (file)
--- a/src/core/store-private.h
+++ b/src/core/store-private.h
@@ -126,6 +126,13 @@ struct sdb_store_expr {
 
        sdb_data_t data;
 };
+#define EXPR_TO_STRING(e) \
+       (((e)->type == TYPED_EXPR) ? "<typed>" \
+               : ((e)->type == ATTR_VALUE) ? "attribute" \
+               : ((e)->type == FIELD_VALUE) ? SDB_FIELD_TO_NAME((e)->data.data.integer) \
+               : ((e)->type == 0) ? "<constant>" \
+               : ((e)->type > 0) ? SDB_DATA_OP_TO_STRING((e)->type) \
+               : "<unknown>")
 
 /*
  * matchers

diff --git a/src/frontend/analyzer.c b/src/frontend/analyzer.c
index 701130e88d498335c13ccc133719bca5989cda4a..8d228c47e17d2b30d8ffb5ca95813cb8dc740e9e 100644 (file)
--- a/src/frontend/analyzer.c
+++ b/src/frontend/analyzer.c
@@ -68,6 +68,47 @@ cmp_error(sdb_strbuf_t *errbuf, int op, int left, int right)
                        SDB_TYPE_TO_STRING(right));
 } /* cmp_error */
 
+static int
+analyze_expr(int context, sdb_store_expr_t *e, sdb_strbuf_t *errbuf)
+{
+       if (! e)
+               return 0;
+
+       if ((e->type < TYPED_EXPR) || (SDB_DATA_CONCAT < e->type)) {
+               sdb_strbuf_sprintf(errbuf, "Invalid expression of type %d", e->type);
+               return -1;
+       }
+
+       switch (e->type) {
+               case TYPED_EXPR:
+                       if (analyze_expr((int)e->data.data.integer, e->left, errbuf))
+                               return -1;
+                       if (context == (int)e->data.data.integer)
+                               return 0;
+                       if ((e->data.data.integer == SDB_HOST) &&
+                                       ((context == SDB_SERVICE) || (context == SDB_METRIC)))
+                               return 0;
+                       sdb_strbuf_sprintf(errbuf, "Invalid expression %s.%s "
+                                       "in %s context",
+                                       SDB_STORE_TYPE_TO_NAME(e->data.data.integer),
+                                       EXPR_TO_STRING(e->left), SDB_STORE_TYPE_TO_NAME(context));
+                       return -1;
+
+               case ATTR_VALUE:
+               case FIELD_VALUE:
+               case 0:
+                       break;
+
+               default:
+                       if (analyze_expr(context, e->left, errbuf))
+                               return -1;
+                       if (analyze_expr(context, e->right, errbuf))
+                               return -1;
+                       break;
+       }
+       return 0;
+} /* analyze_expr */
+
 static int
 analyze_matcher(int context, sdb_store_matcher_t *m, sdb_strbuf_t *errbuf)
 {
@@ -165,6 +206,11 @@ analyze_matcher(int context, sdb_store_matcher_t *m, sdb_strbuf_t *errbuf)
                case MATCHER_GE:
                case MATCHER_GT:
                        assert(CMP_M(m)->left && CMP_M(m)->right);
+                       if (analyze_expr(context, CMP_M(m)->left, errbuf))
+                               return -1;
+                       if (analyze_expr(context, CMP_M(m)->right, errbuf))
+                               return -1;
+
                        if ((CMP_M(m)->left->data_type > 0)
                                        && (CMP_M(m)->right->data_type > 0)
                                        && (CMP_M(m)->left->data_type == CMP_M(m)->right->data_type))
@@ -184,6 +230,11 @@ analyze_matcher(int context, sdb_store_matcher_t *m, sdb_strbuf_t *errbuf)
                        break;
 
                case MATCHER_IN:
+                       if (analyze_expr(context, CMP_M(m)->left, errbuf))
+                               return -1;
+                       if (analyze_expr(context, CMP_M(m)->right, errbuf))
+                               return -1;
+
                        if ((CMP_M(m)->left->data_type > 0)
                                        && (CMP_M(m)->left->data_type & SDB_TYPE_ARRAY)) {
                                cmp_error(errbuf, m->type, CMP_M(m)->left->data_type,
@@ -200,6 +251,11 @@ analyze_matcher(int context, sdb_store_matcher_t *m, sdb_strbuf_t *errbuf)
 
                case MATCHER_REGEX:
                case MATCHER_NREGEX:
+                       if (analyze_expr(context, CMP_M(m)->left, errbuf))
+                               return -1;
+                       if (analyze_expr(context, CMP_M(m)->right, errbuf))
+                               return -1;
+
                        /* all types are supported for the left operand */
                        if ((CMP_M(m)->right->data_type > 0)
                                        && (CMP_M(m)->right->data_type != SDB_TYPE_REGEX)
@@ -212,6 +268,8 @@ analyze_matcher(int context, sdb_store_matcher_t *m, sdb_strbuf_t *errbuf)
 
                case MATCHER_ISNULL:
                case MATCHER_ISNNULL:
+                       if (analyze_expr(context, ISNULL_M(m)->expr, errbuf))
+                               return -1;
                        break;
 
                default:

diff --git a/t/unit/core/store_lookup_test.c b/t/unit/core/store_lookup_test.c
index 49f9125647dafbd17fa630278f749a1d630aa03b..b8b8d7d4ecb1f275e12a907cf3b8b51ac53be1d7 100644 (file)
--- a/t/unit/core/store_lookup_test.c
+++ b/t/unit/core/store_lookup_test.c
@@ -575,6 +575,11 @@ START_TEST(test_scan)
                { "ANY attribute = 'x'", NULL,         0 },
                { "ANY attribute =~ 'x'", NULL,        0 },
                { "ALL attribute = 'k1'", NULL,        2 },
+               { "host.name = 'a'", NULL,             1 },
+               { "host.attribute['k1'] =~ 'v1'",
+                       NULL,                              1 },
+               { "host.attribute['x1'] IS NULL",
+                       NULL,                              3 },
                { "attribute['k1'] = 'v1'", NULL,      1 },
                { "attribute['k1'] = 'v1'",
                        "name != 'k1'",                    0 },

diff --git a/t/unit/frontend/parser_test.c b/t/unit/frontend/parser_test.c
index c5b514fde5156fa43fafed5a714e6ffbd2e768cf..0de4d256b3e37e2616af90eee6d85907462abc46 100644 (file)
--- a/t/unit/frontend/parser_test.c
+++ b/t/unit/frontend/parser_test.c
@@ -107,12 +107,22 @@ START_TEST(test_parse)
                  "name =~ 'p' "
                  "FILTER age>"
                  "interval",           -1,   1, SDB_CONNECTION_LOOKUP },
+               { "LOOKUP hosts MATCHING "
+                 "host.name =~ 'p'",   -1,   1, SDB_CONNECTION_LOOKUP },
                { "LOOKUP services",    -1,   1, SDB_CONNECTION_LOOKUP },
                { "LOOKUP services MATCHING ANY "
                  "attribute =~ 'a'",   -1,   1, SDB_CONNECTION_LOOKUP },
+               { "LOOKUP services MATCHING "
+                 "host.name = 'p'",    -1,   1, SDB_CONNECTION_LOOKUP },
+               { "LOOKUP services MATCHING "
+                 "service.name = 'p'", -1,   1, SDB_CONNECTION_LOOKUP },
                { "LOOKUP metrics",     -1,   1, SDB_CONNECTION_LOOKUP },
                { "LOOKUP metrics MATCHING ANY "
                  "attribute =~ 'a'",   -1,   1, SDB_CONNECTION_LOOKUP },
+               { "LOOKUP metrics MATCHING "
+                 "host.name = 'p'",    -1,   1, SDB_CONNECTION_LOOKUP },
+               { "LOOKUP metrics MATCHING "
+                 "metric.name = 'p'",  -1,   1, SDB_CONNECTION_LOOKUP },
 
                { "TIMESERIES 'host'.'metric' "
                  "START 2014-01-01 "
@@ -276,18 +286,24 @@ START_TEST(test_parse)
                  "'f' || oo",           -1, -1, 0 },
                { "LOOKUP hosts MATCHING "
                  "ANY host = 'host'",   -1, -1, 0 },
+               { "LOOKUP hosts MATCHING "
+                 "service.name = 's'",  -1, -1, 0 },
                { "LOOKUP services MATCHING "
                  "ANY host = 'host'",   -1, -1, 0 },
                { "LOOKUP services MATCHING "
                  "ANY service = 'svc'", -1, -1, 0 },
                { "LOOKUP services MATCHING "
                  "ANY metric = 'm'",    -1, -1, 0 },
+               { "LOOKUP services MATCHING "
+                 "metric.name = 'm'",   -1, -1, 0 },
                { "LOOKUP metrics MATCHING "
                  "ANY host = 'host'",   -1, -1, 0 },
                { "LOOKUP metrics MATCHING "
                  "ANY service = 'svc'", -1, -1, 0 },
                { "LOOKUP metrics MATCHING "
                  "ANY metric = 'm'",    -1, -1, 0 },
+               { "LOOKUP metrics MATCHING "
+                 "service.name = 'm'",   -1, -1, 0 },
        };
 
        sdb_strbuf_t *errbuf = sdb_strbuf_create(64);