From 2a5aff78eb32778a27b707aa99a2494b1371a5dd Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Fri, 8 May 2015 18:49:59 +0200 Subject: [PATCH] parser/analyzer: Fixed iterator type checks. Also, added some more test-cases covering affected cases. --- src/parser/analyzer.c | 38 +++++++++++++++++++++---------------- t/unit/parser/parser_test.c | 36 ++++++++++++++++++++++++++++++----- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/parser/analyzer.c b/src/parser/analyzer.c index 273463d..01539ed 100644 --- a/src/parser/analyzer.c +++ b/src/parser/analyzer.c @@ -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 */ diff --git a/t/unit/parser/parser_test.c b/t/unit/parser/parser_test.c index f904921..cfc36fc 100644 --- a/t/unit/parser/parser_test.c +++ b/t/unit/parser/parser_test.c @@ -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 }, -- 2.30.2