From: Sebastian Harl Date: Sun, 8 Mar 2015 16:15:33 +0000 (+0100) Subject: Let iterator operators fill in the left operand of child operators. X-Git-Tag: sysdb-0.8.0~133 X-Git-Url: https://git.tokkee.org/?p=sysdb.git;a=commitdiff_plain;h=59947b2a42517a95b9c4451c8ebca0c0d8aadc11;hp=b072d9b2786422fcb8f068dd5076bd108a6228e9 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 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;