Code

parser/analyzer: Fixed iterator type checks.
authorSebastian Harl <sh@tokkee.org>
Fri, 8 May 2015 16:49:59 +0000 (18:49 +0200)
committerSebastian Harl <sh@tokkee.org>
Fri, 8 May 2015 16:49:59 +0000 (18:49 +0200)
Also, added some more test-cases covering affected cases.

src/parser/analyzer.c
t/unit/parser/parser_test.c

index 273463d8ce3a2037d2e7211dbb9113c9eec30676..01539edbe71f870de8486b52320caa6dd9fa4444 100644 (file)
@@ -217,23 +217,17 @@ analyze_iter(int context, sdb_ast_iter_t *iter, sdb_strbuf_t *errbuf)
 
        if (analyze_node(iter_context, iter->iter, errbuf))
                return -1;
-       if (iter->iter->data_type > 0)
-               c.value.type = iter->iter->data_type & 0xff;
-       else
-               c.value.type = -1;
-
        /* TODO: support other setups as well */
        assert((iter->expr->type == SDB_AST_TYPE_OPERATOR)
                        && (! SDB_AST_OP(iter->expr)->left));
-       SDB_AST_OP(iter->expr)->left = SDB_AST_NODE(&c);
-       status = analyze_node(context, iter->expr, errbuf);
-       SDB_AST_OP(iter->expr)->left = NULL;
-       if (status)
-               return -1;
+       /* determine the data-type for better error messages */
+       analyze_node(iter_context, SDB_AST_OP(iter->expr)->right, NULL);
 
        if (iter->iter->type == SDB_AST_TYPE_TYPED) {
                int iter_type = SDB_AST_TYPED(iter->iter)->type;
 
+               c.value.type = iter->iter->data_type;
+
                if (iter_type == SDB_ATTRIBUTE) {
                        /* attributes are always iterable */
                }
@@ -262,12 +256,9 @@ analyze_iter(int context, sdb_ast_iter_t *iter, sdb_strbuf_t *errbuf)
        else if (iter->iter->type == SDB_AST_TYPE_VALUE) {
                int iter_type = SDB_AST_VALUE(iter->iter)->type;
 
-               if (iter_type == SDB_FIELD_BACKEND) {
-                       /* backends are always iterable */
-               }
-               else if ((context != SDB_HOST) && (context != SDB_SERVICE)
-                               && (context != SDB_METRIC) && (context != SDB_ATTRIBUTE)
-                               && (context != UNSPEC_CONTEXT)) {
+               c.value.type = iter->iter->data_type & 0xff;
+
+               if (iter_type != SDB_FIELD_BACKEND) {
                        iter_error(errbuf, iter, "%s not iterable in %s context",
                                        (iter_type == SDB_ATTRIBUTE)
                                                ? "attribute"
@@ -277,12 +268,27 @@ analyze_iter(int context, sdb_ast_iter_t *iter, sdb_strbuf_t *errbuf)
                }
        }
        else if (iter->iter->type == SDB_AST_TYPE_CONST) {
+               c.value.type = iter->iter->data_type & 0xff;
+
                if (! (SDB_AST_CONST(iter->iter)->value.type & SDB_TYPE_ARRAY)) {
                        iter_error(errbuf, iter, "%s not iterable",
                                        SDB_TYPE_TO_STRING(SDB_AST_CONST(iter->iter)->value.type));
                        return -1;
                }
        }
+       else {
+               /* TODO: if we know the data-type of iter->iter and it's an array,
+                * we should support an iterator for it as well */
+               iter_error(errbuf, iter, "%s expression not iterable",
+                               SDB_AST_TYPE_TO_STRING(iter->iter));
+               return -1;
+       }
+
+       SDB_AST_OP(iter->expr)->left = SDB_AST_NODE(&c);
+       status = analyze_node(context, iter->expr, errbuf);
+       SDB_AST_OP(iter->expr)->left = NULL;
+       if (status)
+               return -1;
        return 0;
 } /* analyze_iter */
 
index f9049211440c513c342ce6c3769c7f160f8767bf..cfc36fc36417610175455f3e7bef2a8cc844b7c6 100644 (file)
@@ -357,6 +357,20 @@ struct {
        { "LOOKUP hosts MATCHING "
          "name < ''",           -1,  1, SDB_AST_TYPE_LOOKUP, SDB_HOST },
 
+       /* typed expressions */
+       { "LOOKUP services MATCHING "
+         "host.attribute['a'] = 'a'",
+                                -1,  1, SDB_AST_TYPE_LOOKUP, SDB_SERVICE },
+       /* TODO: this should work but the analyzer currently sees ATTRIBUTE
+        * (instead of SERVICE-ATTRIBUTE) as the child type
+       { "LOOKUP services MATCHING "
+         "ANY attribute.service.name = 's'",
+                                -1,  1, SDB_AST_TYPE_LOOKUP, SDB_SERVICE },
+        */
+       { "LOOKUP hosts MATCHING "
+         "ANY service.service.name = 's'",
+                                -1,  1, SDB_AST_TYPE_LOOKUP, SDB_HOST },
+
        /* NULL */
        { "LOOKUP hosts MATCHING "
          "attribute['foo'] "
@@ -422,6 +436,8 @@ struct {
          "value = 'a'",           -1, -1, 0, 0 },
        { "LIST metrics FILTER "
          "value = 'a'",           -1, -1, 0, 0 },
+       { "LIST metrics FILTER "
+         "name.1 = 'a'",          -1, -1, 0, 0 },
 
        /* type mismatches */
        { "LOOKUP hosts MATCHING "
@@ -431,11 +447,6 @@ struct {
          "1 IN backend ",      -1,  -1, 0, 0 },
        { "LOOKUP hosts MATCHING "
          "1 NOT IN backend ",  -1,  -1, 0, 0 },
-       { "LOOKUP hosts MATCHING "
-         "ANY backend !~ backend",
-                               -1,  -1, 0, 0 },
-       { "LOOKUP hosts MATCHING "
-         "ANY backend = 1",    -1,  -1, 0, 0 },
        { "LOOKUP hosts MATCHING "
          "age > 0",             -1, -1, 0, 0 },
        { "LOOKUP hosts MATCHING "
@@ -518,8 +529,23 @@ struct {
          "name + 1 IS NULL",    -1, -1, 0, 0 },
        { "LOOKUP hosts FILTER "
          "name + 1 IS NULL",    -1, -1, 0, 0 },
+
+       /* invalid iterators */
+       { "LOOKUP hosts MATCHING "
+         "ANY backend !~ backend",
+                               -1,  -1, 0, 0 },
+       { "LOOKUP hosts MATCHING "
+         "ANY backend = 1",    -1,  -1, 0, 0 },
        { "LOOKUP hosts MATCHING "
          "ANY 'patt' =~ 'p'",  -1,  -1, 0, 0 },
+       { "LOOKUP hosts MATCHING "
+         "ALL 1 || '2' < '3'", -1,  -1, 0, 0 },
+       { "LOOKUP hosts MATCHING "
+         "ALL name =~ 'a'",    -1,  -1, 0, 0 },
+       /* this could work in theory but is not supported atm */
+       { "LOOKUP hosts MATCHING "
+         "ANY backend || 'a' = 'b'",
+                               -1,  -1, 0, 0 },
 
        /* invalid LIST commands */
        { "LIST",                -1, -1, 0, 0 },