Let iterator operators fill in the left operand of child operators.

This avoids the need for repeating the same operand in two places and better
reflects what's actually going on.

diff --git a/src/core/store-private.h b/src/core/store-private.h
index cde2216327582968966d6f053adc62174ee0de99..2b6b09662f36a91bad7821ed8baacd152ac7e98c 100644 (file)
--- a/src/core/store-private.h
+++ b/src/core/store-private.h
@@ -126,6 +126,7 @@ struct sdb_store_expr {
 
        sdb_data_t data;
 };
+#define EXPR_INIT { SDB_OBJECT_INIT, 0, -1, NULL, NULL, SDB_DATA_INIT }
 #define EXPR_TO_STRING(e) \
        (((e)->type == TYPED_EXPR) ? "<typed>" \
                : ((e)->type == ATTR_VALUE) ? "attribute" \

diff --git a/src/core/store_lookup.c b/src/core/store_lookup.c
index 94f95cc5188f4c03d146bd1b7157adab3f62f76f..e3b5e8a7331d2953a68e5dfe8ce7f0bcafb0e426 100644 (file)
--- a/src/core/store_lookup.c
+++ b/src/core/store_lookup.c
@@ -193,45 +193,33 @@ match_unary(sdb_store_matcher_t *m, sdb_store_obj_t *obj,
 
 /* iterate arrays: ANY/ALL <array> <cmp> <value> */
 static int
-match_iter_array(sdb_store_matcher_t *m, sdb_store_obj_t *obj,
+match_iter_backends(sdb_store_matcher_t *m, sdb_store_obj_t *obj,
                sdb_store_matcher_t *filter)
 {
-       sdb_store_expr_t *e1, *e2;
-       sdb_data_t v1 = SDB_DATA_INIT;
-       sdb_data_t v2 = SDB_DATA_INIT;
-
+       sdb_data_t op = SDB_DATA_INIT;
        int status;
 
-       assert(CMP_M(m)->left && CMP_M(m)->right);
-
        if ((ITER_M(m)->m->type < MATCHER_LT)
                        || (MATCHER_NREGEX < ITER_M(m)->m->type))
                return 0;
 
-       e1 = CMP_M(ITER_M(m)->m)->left;
-       e2 = CMP_M(ITER_M(m)->m)->right;
-
-       if (expr_eval2(e1, &v1, e2, &v2, obj, filter))
+       if (sdb_store_expr_eval(CMP_M(ITER_M(m)->m)->right, obj, &op, filter))
                return 0;
 
-       if ((! (v1.type & SDB_TYPE_ARRAY)) || (v2.type & SDB_TYPE_ARRAY))
+       if (op.type & SDB_TYPE_ARRAY)
                status = 0;
-       else if (sdb_data_isnull(&v1) || (sdb_data_isnull(&v2)))
+       else if (sdb_data_isnull(&op))
                status = 0;
        else {
                size_t i;
                int all = (int)(m->type == MATCHER_ALL);
 
                status = all;
-               for (i = 0; i < v1.data.array.length; ++i) {
-                       sdb_data_t v = SDB_DATA_INIT;
-                       if (sdb_data_array_get(&v1, i, &v)) {
-                               status = 0;
-                               break;
-                       }
+               for (i = 0; i < obj->backends_num; ++i) {
+                       sdb_data_t v = { SDB_TYPE_STRING, { .string = obj->backends[i] } };
 
-                       if (match_value(ITER_M(m)->m->type, &v, &v2,
-                                               (e1->data_type) < 0 || (e2->data_type < 0))) {
+                       if (match_value(ITER_M(m)->m->type, &v, &op,
+                                               CMP_M(ITER_M(m)->m)->right->data_type < 0)) {
                                if (! all) {
                                        status = 1;
                                        break;
@@ -244,9 +232,9 @@ match_iter_array(sdb_store_matcher_t *m, sdb_store_obj_t *obj,
                }
        }
 
-       expr_free_datum2(e1, &v1, e2, &v2);
+       sdb_data_free_datum(&op);
        return status;
-} /* match_iter_array */
+} /* match_iter_backends */
 
 static int
 match_iter(sdb_store_matcher_t *m, sdb_store_obj_t *obj,
@@ -257,11 +245,12 @@ match_iter(sdb_store_matcher_t *m, sdb_store_obj_t *obj,
        int all = (int)(m->type == MATCHER_ALL);
 
        assert((m->type == MATCHER_ANY) || (m->type == MATCHER_ALL));
+       assert((! CMP_M(ITER_M(m)->m)->left) && CMP_M(ITER_M(m)->m)->right);
 
        if (ITER_M(m)->iter->type == FIELD_VALUE) {
                if (ITER_M(m)->iter->data.data.integer != SDB_FIELD_BACKEND)
                        return 0;
-               return match_iter_array(m, obj, filter);
+               return match_iter_backends(m, obj, filter);
        }
 
        assert(ITER_M(m)->iter->type == TYPED_EXPR);
@@ -285,10 +274,24 @@ match_iter(sdb_store_matcher_t *m, sdb_store_obj_t *obj,
        status = all;
        while (sdb_avltree_iter_has_next(iter)) {
                sdb_store_obj_t *child = STORE_OBJ(sdb_avltree_iter_get_next(iter));
+               sdb_store_expr_t expr = EXPR_INIT;
+               sdb_data_t v = SDB_DATA_INIT;
+               bool matches;
+
                if (filter && (! sdb_store_matcher_matches(filter, child, NULL)))
                        continue;
+               if (sdb_store_expr_eval(ITER_M(m)->iter, child, &v, filter))
+                       continue;
 
-               if (sdb_store_matcher_matches(ITER_M(m)->m, child, filter)) {
+               expr.type = 0;
+               expr.data_type = v.type;
+               expr.data = v;
+
+               CMP_M(ITER_M(m)->m)->left = &expr;
+               matches = sdb_store_matcher_matches(ITER_M(m)->m, child, filter);
+               CMP_M(ITER_M(m)->m)->left = NULL;
+               sdb_data_free_datum(&v);
+               if (matches) {
                        if (! all) {
                                status = 1;
                                break;
@@ -589,6 +592,14 @@ sdb_store_any_matcher(sdb_store_expr_t *iter, sdb_store_matcher_t *m)
                                "(invalid operator)", MATCHER_SYM(m->type));
                return NULL;
        }
+       if (CMP_M(m)->left) {
+               sdb_log(SDB_LOG_ERR, "store: Invalid ANY %s %s %s matcher "
+                               "(invalid left operand)",
+                               SDB_TYPE_TO_STRING(CMP_M(m)->left->data_type),
+                               MATCHER_SYM(m->type),
+                               SDB_TYPE_TO_STRING(CMP_M(m)->right->data_type));
+               return NULL;
+       }
        return M(sdb_object_create("any-matcher", iter_type,
                                MATCHER_ANY, iter, m));
 } /* sdb_store_any_matcher */
@@ -601,6 +612,14 @@ sdb_store_all_matcher(sdb_store_expr_t *iter, sdb_store_matcher_t *m)
                                "(invalid operator)", MATCHER_SYM(m->type));
                return NULL;
        }
+       if (CMP_M(m)->left) {
+               sdb_log(SDB_LOG_ERR, "store: Invalid ALL %s %s %s matcher "
+                               "(invalid left operand)",
+                               SDB_TYPE_TO_STRING(CMP_M(m)->left->data_type),
+                               MATCHER_SYM(m->type),
+                               SDB_TYPE_TO_STRING(CMP_M(m)->right->data_type));
+               return NULL;
+       }
        return M(sdb_object_create("all-matcher", iter_type,
                                MATCHER_ALL, iter, m));
 } /* sdb_store_all_matcher */

diff --git a/src/frontend/analyzer.c b/src/frontend/analyzer.c
index b5d0e0825ce785dd09468d2474676bd7888749b8..c71d5ca078b5fbe9a3915013c183bb05ab6bd1f5 100644 (file)
--- a/src/frontend/analyzer.c
+++ b/src/frontend/analyzer.c
@@ -40,10 +40,11 @@
  */
 
 static void
-iter_error(sdb_strbuf_t *errbuf, int op, int oper, int context)
+iter_error(sdb_strbuf_t *errbuf, int op, sdb_store_expr_t *iter, int context)
 {
-       sdb_strbuf_sprintf(errbuf, "Cannot use %s %s in %s context",
-                       MATCHER_SYM(op), SDB_STORE_TYPE_TO_NAME(oper),
+       sdb_strbuf_sprintf(errbuf, "Invalid %s iterator: %s %s "
+                       "not iterable in %s context", MATCHER_SYM(op),
+                       EXPR_TO_STRING(iter), SDB_STORE_TYPE_TO_NAME(iter->data_type),
                        SDB_STORE_TYPE_TO_NAME(context));
 } /* iter_error */
 
@@ -152,41 +153,51 @@ analyze_matcher(int context, int parent_type,
                case MATCHER_ANY:
                case MATCHER_ALL:
                {
-                       int type;
+                       int type = -1;
+                       int left_type = -1;
+
                        assert(ITER_M(m)->m);
-                       if ((ITER_M(m)->iter->type == TYPED_EXPR)
-                                       || (ITER_M(m)->iter->type == FIELD_VALUE)) {
+
+                       if (ITER_M(m)->iter->type == TYPED_EXPR) {
                                type = (int)ITER_M(m)->iter->data.data.integer;
+                               left_type = ITER_M(m)->iter->data_type;
+                       }
+                       else if (ITER_M(m)->iter->type == FIELD_VALUE) {
+                               type = (int)ITER_M(m)->iter->data.data.integer;
+                               /* element type of the field */
+                               left_type = ITER_M(m)->iter->data_type & 0xff;
                        }
                        else {
-                               iter_error(errbuf, m->type, -1, context);
+                               iter_error(errbuf, m->type, ITER_M(m)->iter, context);
                                return -1;
                        }
+
                        if ((context != SDB_HOST)
                                        && (context != SDB_SERVICE)
                                        && (context != SDB_METRIC)) {
-                               iter_error(errbuf, m->type, type, context);
+                               iter_error(errbuf, m->type, ITER_M(m)->iter, context);
                                return -1;
                        }
                        if (type == context) {
-                               iter_error(errbuf, m->type, type, context);
+                               iter_error(errbuf, m->type, ITER_M(m)->iter, context);
                                return -1;
                        }
                        if ((type != SDB_SERVICE)
                                        && (type != SDB_METRIC)
                                        && (type != SDB_ATTRIBUTE)
                                        && (type != SDB_FIELD_BACKEND)) {
-                               iter_error(errbuf, m->type, type, context);
+                               iter_error(errbuf, m->type, ITER_M(m)->iter, context);
                                return -1;
                        }
                        if ((context == SDB_SERVICE) && (type == SDB_METRIC)) {
-                               iter_error(errbuf, m->type, type, context);
+                               iter_error(errbuf, m->type, ITER_M(m)->iter, context);
                                return -1;
                        }
                        else if ((context == SDB_METRIC) && (type == SDB_SERVICE)) {
-                               iter_error(errbuf, m->type, type, context);
+                               iter_error(errbuf, m->type, ITER_M(m)->iter, context);
                                return -1;
                        }
+
                        /* any ary operator will do but these are the once
                         * we currently support */
                        if ((ITER_M(m)->m->type != MATCHER_LT)
@@ -198,31 +209,20 @@ analyze_matcher(int context, int parent_type,
                                        && (ITER_M(m)->m->type != MATCHER_REGEX)
                                        && (ITER_M(m)->m->type != MATCHER_NREGEX)) {
                                iter_op_error(errbuf, m->type,
-                                               CMP_M(ITER_M(m)->m)->left->data_type,
-                                               ITER_M(m)->m->type,
+                                               left_type, ITER_M(m)->m->type,
                                                CMP_M(ITER_M(m)->m)->right->data_type);
                                return -1;
                        }
-                       if (type == SDB_FIELD_BACKEND) {
-                               if (CMP_M(ITER_M(m)->m)->right->data_type < 0)
-                                       return 0; /* skip further type checks */
-                               if (CMP_M(ITER_M(m)->m)->right->data_type & SDB_TYPE_ARRAY) {
-                                       iter_op_error(errbuf, m->type,
-                                                       CMP_M(ITER_M(m)->m)->left->data_type,
-                                                       ITER_M(m)->m->type,
-                                                       CMP_M(ITER_M(m)->m)->right->data_type);
-                                       return -1;
-                               }
-                               if ((CMP_M(ITER_M(m)->m)->left->data_type & 0xff)
-                                               != CMP_M(ITER_M(m)->m)->right->data_type) {
+                       if ((left_type >= 0)
+                                       && (CMP_M(ITER_M(m)->m)->right->data_type >= 0)) {
+                               if (left_type != CMP_M(ITER_M(m)->m)->right->data_type) {
                                        iter_op_error(errbuf, m->type,
-                                                       CMP_M(ITER_M(m)->m)->left->data_type,
-                                                       ITER_M(m)->m->type,
+                                                       left_type, ITER_M(m)->m->type,
                                                        CMP_M(ITER_M(m)->m)->right->data_type);
                                        return -1;
                                }
                        }
-                       else if (analyze_matcher(type, m->type, ITER_M(m)->m, errbuf))
+                       if (analyze_matcher(type, m->type, ITER_M(m)->m, errbuf))
                                return -1;
                        break;
                }
@@ -233,13 +233,17 @@ analyze_matcher(int context, int parent_type,
                case MATCHER_NE:
                case MATCHER_GE:
                case MATCHER_GT:
+               {
+                       int left_type = -1;
+
                        assert(CMP_M(m)->right);
                        if ((parent_type == MATCHER_ALL)
                                        || (parent_type == MATCHER_ANY)) {
-                               // TODO: assert(! CMP_M(m)->left);
+                               assert(! CMP_M(m)->left);
                        }
                        else {
                                assert(CMP_M(m)->left);
+                               left_type = CMP_M(m)->left->data_type;
                        }
 
                        if (analyze_expr(context, CMP_M(m)->left, errbuf))
@@ -247,27 +251,26 @@ analyze_matcher(int context, int parent_type,
                        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)) {
-                               if (CMP_M(m)->left->data_type == CMP_M(m)->right->data_type)
+                       if ((left_type > 0) && (CMP_M(m)->right->data_type > 0)) {
+                               if (left_type == CMP_M(m)->right->data_type)
                                        return 0;
-                               cmp_error(errbuf, m->type, CMP_M(m)->left->data_type,
+                               cmp_error(errbuf, m->type, left_type,
                                                CMP_M(m)->right->data_type);
                                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,
+                       if ((left_type > 0) && (left_type & SDB_TYPE_ARRAY)) {
+                               cmp_error(errbuf, m->type, left_type,
                                                CMP_M(m)->right->data_type);
                                return -1;
                        }
                        if ((CMP_M(m)->right->data_type > 0)
                                        && (CMP_M(m)->right->data_type & SDB_TYPE_ARRAY)) {
-                               cmp_error(errbuf, m->type, CMP_M(m)->left->data_type,
+                               cmp_error(errbuf, m->type, left_type,
                                                CMP_M(m)->right->data_type);
                                return -1;
                        }
                        break;
+               }
 
                case MATCHER_IN:
                case MATCHER_NIN:

diff --git a/src/frontend/grammar.y b/src/frontend/grammar.y
index ac899e3708212fe78b928fe6977fb86c6d5cfb2b..a573b2b1dac5e425d3febba39cbc10657f399ed6 100644 (file)
--- a/src/frontend/grammar.y
+++ b/src/frontend/grammar.y
@@ -867,7 +867,7 @@ name_iter_matcher(int type, sdb_store_expr_t *iter, const char *cmp,
        sdb_store_matcher_t *m, *tmp = NULL;
        assert(cb);
 
-       m = cb(iter, expr);
+       m = cb(NULL, expr);
        if (type == MATCHER_ANY)
                tmp = sdb_store_any_matcher(iter, m);
        else if (type == MATCHER_ALL)

diff --git a/t/unit/core/store_lookup_test.c b/t/unit/core/store_lookup_test.c
index 642af72d92bffed59364085e6556d673cddcdf6c..923ce066e498b90ab41e7754020e04d3d4dd55ff 100644 (file)
--- a/t/unit/core/store_lookup_test.c
+++ b/t/unit/core/store_lookup_test.c
@@ -152,7 +152,7 @@ START_TEST(test_cmp_name)
 {
        sdb_store_obj_t *host;
        sdb_data_t datum;
-       sdb_store_expr_t *obj, *value;
+       sdb_store_expr_t *obj = NULL, *value;
        sdb_store_matcher_t *m, *n;
        int status;
 
@@ -163,10 +163,12 @@ START_TEST(test_cmp_name)
        datum.type = SDB_TYPE_STRING;
        datum.data.string = cmp_name_data[_i].name;
 
-       obj = sdb_store_expr_fieldvalue(SDB_FIELD_NAME);
-       fail_unless(obj != NULL,
-                       "sdb_store_expr_fieldvalue(SDB_STORE_NAME) = NULL; "
-                       "expected: <expr>");
+       if (cmp_name_data[_i].type == SDB_HOST) {
+               obj = sdb_store_expr_fieldvalue(SDB_FIELD_NAME);
+               fail_unless(obj != NULL,
+                               "sdb_store_expr_fieldvalue(SDB_STORE_NAME) = NULL; "
+                               "expected: <expr>");
+       }
        value = sdb_store_expr_constvalue(&datum);
        fail_unless(value != NULL,
                        "sdb_store_expr_constvalue(%s) = NULL; "
@@ -176,11 +178,14 @@ START_TEST(test_cmp_name)
                m = sdb_store_regex_matcher(obj, value);
        else
                m = sdb_store_eq_matcher(obj, value);
+
        if (cmp_name_data[_i].type != SDB_HOST) {
                sdb_store_expr_t *iter;
                sdb_store_matcher_t *tmp;
+               obj = sdb_store_expr_fieldvalue(SDB_FIELD_NAME);
                iter = sdb_store_expr_typed(cmp_name_data[_i].type, obj);
                tmp = sdb_store_any_matcher(iter, m);
+               ck_assert(iter && m);
                sdb_object_deref(SDB_OBJ(iter));
                sdb_object_deref(SDB_OBJ(m));
                m = tmp;