From: Sebastian Harl <sh@tokkee.org>
Date: Fri, 8 May 2015 16:49:59 +0000 (+0200)
Subject: parser/analyzer: Fixed iterator type checks.
X-Git-Tag: sysdb-0.8.0~100
X-Git-Url: https://git.tokkee.org/?p=sysdb.git;a=commitdiff_plain;h=2a5aff78eb32778a27b707aa99a2494b1371a5dd;hp=a46915cabd4fd110fb20ba7bf09cfe1fb14456e1

parser/analyzer: Fixed iterator type checks.

Also, added some more test-cases covering affected cases.
---

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 },