From 59947b2a42517a95b9c4451c8ebca0c0d8aadc11 Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Sun, 8 Mar 2015 17:15:33 +0100 Subject: [PATCH] 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. --- src/core/store-private.h | 1 + src/core/store_lookup.c | 69 +++++++++++++++++----------- src/frontend/analyzer.c | 79 +++++++++++++++++---------------- src/frontend/grammar.y | 2 +- t/unit/core/store_lookup_test.c | 15 ++++--- 5 files changed, 97 insertions(+), 69 deletions(-) diff --git a/src/core/store-private.h b/src/core/store-private.h index cde2216..2b6b096 100644 --- 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) ? "" \ : ((e)->type == ATTR_VALUE) ? "attribute" \ diff --git a/src/core/store_lookup.c b/src/core/store_lookup.c index 94f95cc..e3b5e8a 100644 --- 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 */ 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 b5d0e08..c71d5ca 100644 --- 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 ac899e3..a573b2b 100644 --- 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 642af72..923ce06 100644 --- 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: "); + 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: "); + } 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; -- 2.30.2