From: Sebastian Harl <sh@tokkee.org>
Date: Fri, 17 Oct 2014 17:50:34 +0000 (+0200)
Subject: store, frontend: Make IS (NOT) NULL an unary operator on expressions.
X-Git-Tag: sysdb-0.6.0~105
X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=10a65c7e584820b403567bc849581bb0c3176ff5;p=sysdb.git

store, frontend: Make IS (NOT) NULL an unary operator on expressions.

This is more flexible and in-line with all other operators being migrated to
expression-based constructs.
---

diff --git a/src/core/store-private.h b/src/core/store-private.h
index a64c220..3db771b 100644
--- a/src/core/store-private.h
+++ b/src/core/store-private.h
@@ -181,6 +181,7 @@ enum {
 	MATCHER_REGEX,
 	MATCHER_NREGEX,
 	MATCHER_ISNULL,
+	MATCHER_ISNNULL,
 };
 
 #define MATCHER_SYM(t) \
@@ -201,6 +202,7 @@ enum {
 		: ((t) == MATCHER_REGEX) ? "=~" \
 		: ((t) == MATCHER_NREGEX) ? "!~" \
 		: ((t) == MATCHER_ISNULL) ? "IS NULL" \
+		: ((t) == MATCHER_ISNNULL) ? "IS NOT NULL" \
 		: "UNKNOWN")
 
 /* match the name of something */
@@ -272,7 +274,7 @@ typedef struct {
 
 typedef struct {
 	sdb_store_matcher_t super;
-	char *attr_name; /* we only support matching attributes */
+	sdb_store_expr_t *expr;
 } isnull_matcher_t;
 #define ISNULL_M(m) ((isnull_matcher_t *)(m))
 
diff --git a/src/core/store_lookup.c b/src/core/store_lookup.c
index 325d67f..21e0ba4 100644
--- a/src/core/store_lookup.c
+++ b/src/core/store_lookup.c
@@ -528,10 +528,23 @@ static int
 match_isnull(sdb_store_matcher_t *m, sdb_store_obj_t *obj,
 		sdb_store_matcher_t *filter)
 {
-	assert(m->type == MATCHER_ISNULL);
-	if (obj->type != SDB_HOST)
-		return 0;
-	return attr_get(HOST(obj), ISNULL_M(m)->attr_name, filter) == NULL;
+	sdb_data_t v = SDB_DATA_INIT;
+	int status;
+
+	assert((m->type == MATCHER_ISNULL) || (m->type == MATCHER_ISNNULL));
+
+	/* TODO: this might hide real errors;
+	 * improve error reporting and propagation */
+	if (sdb_store_expr_eval(ISNULL_M(m)->expr, obj, &v, filter)
+			|| sdb_data_isnull(&v))
+		status = 1;
+	else
+		status = 0;
+
+	sdb_data_free_datum(&v);
+	if (m->type == MATCHER_ISNNULL)
+		return !status;
+	return status;
 } /* match_isnull */
 
 typedef int (*matcher_cb)(sdb_store_matcher_t *, sdb_store_obj_t *,
@@ -563,6 +576,7 @@ matchers[] = {
 	match_regex,
 	match_regex,
 	match_isnull,
+	match_isnull,
 };
 
 /*
@@ -950,33 +964,30 @@ uop_tostring(sdb_store_matcher_t *m, char *buf, size_t buflen)
 static int
 isnull_matcher_init(sdb_object_t *obj, va_list ap)
 {
-	const char *name;
-
 	M(obj)->type = va_arg(ap, int);
-	if (M(obj)->type != MATCHER_ISNULL)
+	if ((M(obj)->type != MATCHER_ISNULL) && (M(obj)->type != MATCHER_ISNNULL))
 		return -1;
 
-	name = va_arg(ap, const char *);
-	if (! name)
-		return -1;
-	ISNULL_M(obj)->attr_name = strdup(name);
-	if (! ISNULL_M(obj)->attr_name)
-		return -1;
+	ISNULL_M(obj)->expr = va_arg(ap, sdb_store_expr_t *);
+	sdb_object_ref(SDB_OBJ(ISNULL_M(obj)->expr));
 	return 0;
 } /* isnull_matcher_init */
 
 static void
 isnull_matcher_destroy(sdb_object_t *obj)
 {
-	if (ISNULL_M(obj)->attr_name)
-		free(ISNULL_M(obj)->attr_name);
-	ISNULL_M(obj)->attr_name = NULL;
+	sdb_object_deref(SDB_OBJ(ISNULL_M(obj)->expr));
+	ISNULL_M(obj)->expr = NULL;
 } /* isnull_matcher_destroy */
 
 static char *
 isnull_tostring(sdb_store_matcher_t *m, char *buf, size_t buflen)
 {
-	snprintf(buf, buflen, "(IS NULL, ATTR[%s])", ISNULL_M(m)->attr_name);
+	/* XXX */
+	if (m->type == MATCHER_ISNULL)
+		strncpy(buf, "(IS NULL)", buflen);
+	else
+		strncpy(buf, "(IS NOT NULL)", buflen);
 	return buf;
 } /* isnull_tostring */
 
@@ -1056,6 +1067,7 @@ matchers_tostring[] = {
 	cmp_tostring,
 	cmp_tostring,
 	isnull_tostring,
+	isnull_tostring,
 };
 
 /*
@@ -1236,12 +1248,19 @@ sdb_store_nregex_matcher(sdb_store_expr_t *left, sdb_store_expr_t *right)
 } /* sdb_store_nregex_matcher */
 
 sdb_store_matcher_t *
-sdb_store_isnull_matcher(const char *attr_name)
+sdb_store_isnull_matcher(sdb_store_expr_t *expr)
 {
 	return M(sdb_object_create("isnull-matcher", isnull_type,
-				MATCHER_ISNULL, attr_name));
+				MATCHER_ISNULL, expr));
 } /* sdb_store_isnull_matcher */
 
+sdb_store_matcher_t *
+sdb_store_isnnull_matcher(sdb_store_expr_t *expr)
+{
+	return M(sdb_object_create("isnull-matcher", isnull_type,
+				MATCHER_ISNNULL, expr));
+} /* sdb_store_isnnull_matcher */
+
 sdb_store_matcher_op_cb
 sdb_store_parse_matcher_op(const char *op)
 {
@@ -1341,13 +1360,7 @@ parse_attr_cmp(const char *attr, const char *op, sdb_store_expr_t *expr)
 	if (! attr)
 		return NULL;
 
-	if (! strcasecmp(op, "IS")) {
-		if (! expr)
-			return sdb_store_isnull_matcher(attr);
-		else
-			return NULL;
-	}
-	else if (! expr)
+	if (! expr)
 		return NULL;
 	else if (parse_cond_op(op, &matcher, &inv))
 		return NULL;
diff --git a/src/frontend/grammar.y b/src/frontend/grammar.y
index 1926718..24a1375 100644
--- a/src/frontend/grammar.y
+++ b/src/frontend/grammar.y
@@ -460,21 +460,44 @@ compare_matcher:
 	|
 	IDENTIFIER '[' IDENTIFIER ']' IS NULL_T
 		{
-			$$ = sdb_store_matcher_parse_cmp($1, $3, "IS", NULL);
+			sdb_store_expr_t *expr;
+
+			if (strcasecmp($1, "attribute")) {
+				char errmsg[strlen($1) + strlen($3) + 32];
+				snprintf(errmsg, sizeof(errmsg),
+						YY_("unknown value %s[%s]"), $1);
+				sdb_fe_yyerror(&yylloc, scanner, errmsg);
+				free($1); $1 = NULL;
+				free($3); $3 = NULL;
+				YYABORT;
+			}
+
+			expr = sdb_store_expr_attrvalue($3);
+			$$ = sdb_store_isnull_matcher(expr);
+			sdb_object_deref(SDB_OBJ(expr));
 			free($1); $1 = NULL;
 			free($3); $3 = NULL;
 		}
 	|
 	IDENTIFIER '[' IDENTIFIER ']' IS NOT NULL_T
 		{
-			sdb_store_matcher_t *m;
-			m = sdb_store_matcher_parse_cmp($1, $3, "IS", NULL);
+			sdb_store_expr_t *expr;
+
+			if (strcasecmp($1, "attribute")) {
+				char errmsg[strlen($1) + strlen($3) + 32];
+				snprintf(errmsg, sizeof(errmsg),
+						YY_("unknown value %s[%s]"), $1);
+				sdb_fe_yyerror(&yylloc, scanner, errmsg);
+				free($1); $1 = NULL;
+				free($3); $3 = NULL;
+				YYABORT;
+			}
+
+			expr = sdb_store_expr_attrvalue($3);
+			$$ = sdb_store_isnnull_matcher(expr);
+			sdb_object_deref(SDB_OBJ(expr));
 			free($1); $1 = NULL;
 			free($3); $3 = NULL;
-
-			/* sdb_store_inv_matcher return NULL if m==NULL */
-			$$ = sdb_store_inv_matcher(m);
-			sdb_object_deref(SDB_OBJ(m));
 		}
 	;
 
diff --git a/src/include/core/store.h b/src/include/core/store.h
index 147d745..a7804dd 100644
--- a/src/include/core/store.h
+++ b/src/include/core/store.h
@@ -406,10 +406,17 @@ sdb_store_attr_matcher(const char *name, const char *value, _Bool re);
 
 /*
  * sdb_store_isnull_matcher:
- * Creates a matcher matching "missing" attributes.
+ * Creates a matcher matching NULL values.
  */
 sdb_store_matcher_t *
-sdb_store_isnull_matcher(const char *attr_name);
+sdb_store_isnull_matcher(sdb_store_expr_t *expr);
+
+/*
+ * sdb_store_isnnull_matcher:
+ * Creates a matcher matching non-NULL values.
+ */
+sdb_store_matcher_t *
+sdb_store_isnnull_matcher(sdb_store_expr_t *expr);
 
 /*
  * sdb_store_child_matcher:
diff --git a/t/unit/core/store_lookup_test.c b/t/unit/core/store_lookup_test.c
index 3b0e956..2e1e6d8 100644
--- a/t/unit/core/store_lookup_test.c
+++ b/t/unit/core/store_lookup_test.c
@@ -624,8 +624,6 @@ START_TEST(test_parse_cmp)
 /*		{ "attribute", "attr", "=",  &attrname,   MATCHER_EQ }, */
 		{ "attribute", "attr", ">=", &attrname,   MATCHER_GE },
 		{ "attribute", "attr", ">",  &attrname,   MATCHER_GT },
-		{ "attribute", "attr", "IS", NULL,        MATCHER_ISNULL },
-		{ "attribute", "attr", "IS", &attrname,   -1 },
 		{ "foo",       NULL,   "=",  &attrname,   -1 },
 		{ "foo",       "attr", "=",  &attrname,   -1 },
 	};
@@ -657,7 +655,7 @@ START_TEST(test_parse_cmp)
 
 		fail_unless(check != NULL,
 				"sdb_store_matcher_parse_cmp(%s, %s, %s, %s) = %p; "
-				"expected: NULL", golden_data[i].obj_type,
+				"expected: <expr>", golden_data[i].obj_type,
 				golden_data[i].attr, golden_data[i].op, buf, check);
 		fail_unless(M(check)->type == golden_data[i].expected,
 				"sdb_store_matcher_parse_cmp(%s, %s, %s, %s) returned matcher "
@@ -834,13 +832,13 @@ START_TEST(test_scan)
 		{ "attribute[k1] = 'v1'", NULL,      1,
 			"ATTR\\[k1\\]\\{ VALUE\\{ 'v1', \\(nil\\) \\} \\}" },
 		{ "attribute[k1] IS NULL", NULL,     2,
-			"\\(IS NULL, ATTR\\[k1\\]\\)" },
+			"\\(IS NULL\\)" },
 		{ "attribute[x1] IS NULL", NULL,     3,
-			"\\(IS NULL, ATTR\\[x1\\]\\)" },
+			"\\(IS NULL\\)" },
 		{ "attribute[k1] IS NOT NULL", NULL, 1,
-			"\\(NOT, \\(IS NULL, ATTR\\[k1\\]\\)\\)" },
+			"\\(IS NOT NULL\\)" },
 		{ "attribute[x1] IS NOT NULL", NULL, 0,
-			"\\(NOT, \\(IS NULL, ATTR\\[x1\\]\\)\\)" },
+			"\\(IS NOT NULL\\)" },
 		{ "attribute[k2] < 123", NULL,       0,
 			"ATTR\\[k2\\]\\{ < 123 \\}" },
 		{ "attribute[k2] <= 123", NULL,      1,
diff --git a/t/unit/frontend/parser_test.c b/t/unit/frontend/parser_test.c
index c0cbff8..7839ee8 100644
--- a/t/unit/frontend/parser_test.c
+++ b/t/unit/frontend/parser_test.c
@@ -348,7 +348,7 @@ START_TEST(test_parse_matcher)
 		/* NULL; while this is an implementation detail,
 		 * IS NULL currently maps to an equality matcher */
 		{ "attribute[foo] IS NULL",     -1,  MATCHER_ISNULL },
-		{ "attribute[foo] IS NOT NULL", -1,  MATCHER_NOT },
+		{ "attribute[foo] IS NOT NULL", -1,  MATCHER_ISNNULL },
 
 		/* object field matchers */
 		{ ".last_update < 10s",         -1,  MATCHER_LT },