RedisGraph · nafraf · Apr 12, 2023 · Apr 12, 2023 · Apr 17, 2023 · Apr 17, 2023
diff --git a/src/arithmetic/arithmetic_expression.c b/src/arithmetic/arithmetic_expression.c
@@ -857,12 +857,16 @@ void _AR_EXP_ToString(const AR_ExpNode *root, char **str, size_t *str_size,
 		char binary_op = 0;
 
 		const char *func_name = AR_EXP_GetFuncName(root);
-		if(strcmp(func_name, "ADD") == 0) binary_op = '+';
-		else if(strcmp(func_name, "SUB") == 0) binary_op = '-';
-		else if(strcmp(func_name, "MUL") == 0) binary_op = '*';
-		else if(strcmp(func_name, "DIV") == 0)  binary_op = '/';
+		if(strcasecmp(func_name, "EQ") == 0)  binary_op = '=';
+		else if(strcasecmp(func_name, "ADD") == 0) binary_op = '+';
+		else if(strcasecmp(func_name, "SUB") == 0) binary_op = '-';
+		else if(strcasecmp(func_name, "MUL") == 0) binary_op = '*';
+		else if(strcasecmp(func_name, "DIV") == 0) binary_op = '/';
+		else if(strcasecmp(func_name, "MOD") == 0) binary_op = '%';
+		else if(strcasecmp(func_name, "POW") == 0) binary_op = '^';
 
 		if(binary_op) {
+			*bytes_written += sprintf((*str + *bytes_written), "(");
 			_AR_EXP_ToString(root->op.children[0], str, str_size, bytes_written);
 
 			/* Make sure there are at least 64 bytes in str. */
@@ -872,30 +876,49 @@ void _AR_EXP_ToString(const AR_ExpNode *root, char **str, size_t *str_size,
 			}
 
 			*bytes_written += sprintf((*str + *bytes_written), " %c ", binary_op);
-
 			_AR_EXP_ToString(root->op.children[1], str, str_size, bytes_written);
+			*bytes_written += sprintf((*str + *bytes_written), ")");
 		} else {
-			/* Operation isn't necessarily a binary operation, use function call representation. */
-			*bytes_written += sprintf((*str + *bytes_written), "%s(", AR_EXP_GetFuncName(root));
-
-			for(int i = 0; i < root->op.child_count ; i++) {
-				_AR_EXP_ToString(root->op.children[i], str, str_size, bytes_written);
+			if(strcmp(func_name, "property") == 0) {
+				// For property operation don't use the function call representation
+				// to get a more readable string, like: a.prop
 
 				/* Make sure there are at least 64 bytes in str. */
 				if((*str_size - strlen(*str)) < 64) {
 					*str_size += 128;
 					*str = rm_realloc(*str, sizeof(char) * *str_size);
 				}
-				if(i < (root->op.child_count - 1)) {
-					*bytes_written += sprintf((*str + *bytes_written), ",");
+				_AR_EXP_ToString(root->op.children[0], str, str_size, bytes_written);
+				*bytes_written += sprintf((*str + *bytes_written), ".");
+				SIValue_ToString(root->op.children[1]->operand.constant, str, str_size, bytes_written);
+			} else {
+				/* Operation isn't necessarily a binary operation, use function call representation. */
+				*bytes_written += sprintf((*str + *bytes_written), "%s(", func_name);
+				for(int i = 0; i < root->op.child_count ; i++) {
+					_AR_EXP_ToString(root->op.children[i], str, str_size, bytes_written);
+
+					/* Make sure there are at least 64 bytes in str. */
+					if((*str_size - strlen(*str)) < 64) {
+						*str_size += 128;
+						*str = rm_realloc(*str, sizeof(char) * *str_size);
+					}
+					if(i < (root->op.child_count - 1)) {
+						*bytes_written += sprintf((*str + *bytes_written), ", ");
+					}
 				}
+				*bytes_written += sprintf((*str + *bytes_written), ")");
 			}
-			*bytes_written += sprintf((*str + *bytes_written), ")");
 		}
 	} else {
 		// Concat Operand node.
 		if(root->operand.type == AR_EXP_CONSTANT) {
+			if(root->operand.constant.type == T_STRING) {
+				*bytes_written += sprintf((*str + *bytes_written), "'");
+			}
 			SIValue_ToString(root->operand.constant, str, str_size, bytes_written);
+			if(root->operand.constant.type == T_STRING) {
+				*bytes_written += sprintf((*str + *bytes_written), "'");
+			}
 		} else {
 			*bytes_written += sprintf((*str + *bytes_written), "%s", root->operand.variadic.entity_alias);
 		}

diff --git a/src/arithmetic/arithmetic_expression_construct.c b/src/arithmetic/arithmetic_expression_construct.c
@@ -57,6 +57,18 @@ static inline const char *_ASTOpToString(AST_Operator op) {
 	return OpName[op];
 }
 
+// The OpSymbol array is strictly parallel with the AST_Operator enum.
+static const char *OpSymbol[OP_COUNT] = {
+	"UNKNOWN", "NULL", "OR", "XOR", "AND", "NOT", "=", "!=", "<", ">", "<=",  ">=",
+	"+", "-", "*", "/", "%", "^", "CONTAINS", "STARTS WITH",
+	"ENDS WITH", "IN", "IS NULL", "IS NOT NULL", "XNOR"
+};
+
+const char *ASTOpToSymbolString(AST_Operator op) {
+	ASSERT(op < OP_COUNT);
+	return OpSymbol[op];
+}
+
 static AR_ExpNode *AR_EXP_NewOpNodeFromAST(AST_Operator op, uint child_count) {
 	const char *func_name = _ASTOpToString(op);
 	return AR_EXP_NewOpNode(func_name, true, child_count);

diff --git a/src/arithmetic/arithmetic_expression_construct.h b/src/arithmetic/arithmetic_expression_construct.h
@@ -7,8 +7,11 @@
 #pragma once
 
 #include "ast.h"
+#include "../ast/ast_shared.h"
 #include "arithmetic_expression.h"
 
 // Construct arithmetic expression from AST node
 AR_ExpNode *AR_EXP_FromASTNode(const cypher_astnode_t *expr);
 
+// return operator symbol
+const char *ASTOpToSymbolString(AST_Operator op);
diff --git a/src/execution_plan/ops/op_filter.c b/src/execution_plan/ops/op_filter.c
@@ -12,13 +12,24 @@ static Record FilterConsume(OpBase *opBase);
 static OpBase *FilterClone(const ExecutionPlan *plan, const OpBase *opBase);
 static void FilterFree(OpBase *opBase);
 
+static void FilterToString(
+	const OpBase *ctx, 
+	sds *buf
+) {
+	OpFilter *op = (OpFilter *)ctx;
+	char *exp_str = NULL;
+
+	*buf = sdscatprintf(*buf, "%s | ", op->op.name);
+	FilterTree_ToString(op->filterTree, buf);
+}
+
 OpBase *NewFilterOp(const ExecutionPlan *plan, FT_FilterNode *filterTree) {
 	OpFilter *op = rm_malloc(sizeof(OpFilter));
 	op->filterTree = filterTree;
 
 	// Set our Op operations
 	OpBase_Init((OpBase *)op, OPType_FILTER, "Filter", NULL, FilterConsume,
-				NULL, NULL, FilterClone, FilterFree, false, plan);
+				NULL, FilterToString, FilterClone, FilterFree, false, plan);
 
 	return (OpBase *)op;
 }

diff --git a/src/filter_tree/filter_tree.c b/src/filter_tree/filter_tree.c
@@ -13,6 +13,7 @@
 #include "../util/rmalloc.h"
 #include "../ast/ast_shared.h"
 #include "../datatypes/array.h"
+#include "../arithmetic/arithmetic_expression_construct.h"
 
 // forward declarations
 void _FilterTree_DeMorgan
@@ -1149,6 +1150,46 @@ void FilterTree_Print
 	_FilterTree_Print(root, 0);
 }
 
+void FilterTree_ToString
+(
+	const FT_FilterNode *root,
+	sds *buff
+) {
+	ASSERT(root && buff && *buff);
+
+	char *exp = NULL;
+	char *left = NULL;
+	char *right = NULL;
+
+	if(root == NULL) return;
+
+	switch(root->t) {
+		case FT_N_EXP:
+			AR_EXP_ToString(root->exp.exp, &exp);
+			*buff = sdscatprintf(*buff, "%s",  exp);
+			rm_free(exp);
+			break;
+		case FT_N_PRED:
+			AR_EXP_ToString(root->pred.lhs, &left);
+			AR_EXP_ToString(root->pred.rhs, &right);
+			*buff = sdscatprintf(*buff, "%s %s %s", left, ASTOpToSymbolString(root->pred.op), right);
+			rm_free(left);
+			rm_free(right);
+			break;
+		case FT_N_COND:
+			*buff = sdscatprintf(*buff, "(");
+			FilterTree_ToString(LeftChild(root), buff);
+			*buff = sdscatprintf(*buff, " %s ", ASTOpToSymbolString(root->cond.op));
+			FilterTree_ToString(RightChild(root), buff);
+			*buff = sdscatprintf(*buff, ")");
+			break;
+		default:
+			ASSERT(false);
+			break;
+	}
+
+}
+
 void FilterTree_Free
 (
 	FT_FilterNode *root

diff --git a/src/filter_tree/filter_tree.h b/src/filter_tree/filter_tree.h
@@ -8,6 +8,7 @@
 
 #include <stddef.h>
 #include "../redismodule.h"
+#include "../util/sds/sds.h"
 #include "../ast/ast_shared.h"
 #include "../../deps/rax/rax.h"
 #include "../execution_plan/record.h"
@@ -221,3 +222,9 @@ void FilterTree_Free
 	FT_FilterNode *root
 );
 
+// gets a string representation of given filter tree
+void FilterTree_ToString
+(
+	const FT_FilterNode *root,
+	sds *buff
+);
diff --git a/tests/flow/test_path_filter.py b/tests/flow/test_path_filter.py
@@ -2,6 +2,7 @@
 from common import *
 from index_utils import *
 from collections import Counter
+from execution_plan_util import locate_operation
 
 sys.path.append(os.path.dirname(os.path.abspath(__file__)) + '/../..')
 from demo import QueryInfo
@@ -290,7 +291,20 @@ def test14_path_and_predicate_filters(self):
         query = "MATCH (a:L) WHERE (a)-[]->() AND a.x = 'a' return a.x"
         plan_1 = redis_graph.execution_plan(query)
         # The predicate filter should be evaluated between the Apply and Scan ops.
-        self.env.assertTrue(re.search('Semi Apply\s+Filter\s+Node By Label Scan', plan_1))
+        # Results
+        #     Project
+        #         Semi Apply
+        #             Filter | a.x = 'a'
+        #                 Node By Label Scan | (a:L)
+        #             Conditional Traverse | (a)-[@anon_1]->(@anon_0)
+        #                 Argument | record is null
+        plan = redis_graph.explain(query)
+        semiapply = locate_operation(plan.structured_plan, "Semi Apply")
+        self.env.assertTrue(semiapply and len(semiapply.children) == 2)
+        predfilter = locate_operation(semiapply, "Filter")
+        self.env.assertTrue(predfilter and len(predfilter.children) == 1)
+        scan = locate_operation(predfilter, "Node By Label Scan")
+        self.env.assertTrue(scan)
         result_set = redis_graph.query(query)
         expected_result = [['a']]
         self.env.assertEquals(result_set.result_set, expected_result)

diff --git a/tests/flow/test_profile.py b/tests/flow/test_profile.py
@@ -1,4 +1,7 @@
 from common import *
+from execution_plan_util import locate_operation
+from pprint import pprint
+
 
 GRAPH_ID = "profile"
 
@@ -28,7 +31,7 @@ def test01_profile(self):
 
         self.env.assertIn("Results | Records produced: 2", profile)
         self.env.assertIn("Project | Records produced: 2", profile)
-        self.env.assertIn("Filter | Records produced: 2", profile)
+        self.env.assertIn("Filter | p.v > 1 | Records produced: 2", profile)
         self.env.assertIn("Node By Label Scan | (p:Person) | Records produced: 3", profile)
 
     def test02_profile_after_op_reset(self):
@@ -39,3 +42,33 @@ def test02_profile_after_op_reset(self):
         self.env.assertIn("Update | Records produced: 0", profile)
         self.env.assertIn("Conditional Variable Length Traverse | (a)-[@anon_1*1..INF]->(@anon_0) | Records produced: 0", profile)
         self.env.assertIn("Node By Label Scan | (a:L) | Records produced: 0", profile)
+
+    def test03_profile_filter_information(self):
+        query = """
+                MATCH (n)
+                WHERE n:person
+                RETURN n.name
+                """
+        plan = redis_graph.explain(query)
+        filterOp = locate_operation(plan.structured_plan, "Filter")
+        self.env.assertIn("hasLabels(n, [person])", filterOp.args)
+
+        query = """
+                MATCH (a:Actor)-[r:ACTED_IN]-(m:Movie) 
+                WHERE (a.id%2=1)+1 >= 1 
+                RETURN a
+                """
+        plan = redis_graph.explain(query)
+        filterOp = locate_operation(plan.structured_plan, "Filter")
+        self.env.assertIn("(((a.id % 2) = 1) + 1) >= 1", filterOp.args)
+
+        query = """
+                MATCH (a:Actor)-[r:ACTED_IN]-(m:Movie)
+                    WHERE a.name = 'Mark Hamill'
+                        AND (a.name STARTS WITH 'M' OR a.name CONTAINS 'H')
+                RETURN a.name, m.title
+                """
+        plan = redis_graph.explain(query)
+        filterOp = locate_operation(plan.structured_plan, "Filter")
+        self.env.assertIn("(a.name = 'Mark Hamill' AND (starts with(a.name, 'M') OR contains(a.name, 'H')))", filterOp.args)
+
diff --git a/tests/flow/test_with_clause.py b/tests/flow/test_with_clause.py
@@ -1,5 +1,6 @@
-from common import *
 import re
+from common import *
+from execution_plan_util import locate_operation
 
 redis_graph = None
 values = ["str1", "str2", False, True, 5, 10.5]
@@ -224,22 +225,32 @@ def test10_filter_placement_validate_scopes(self):
         expected = [] # No results should be returned
         self.env.assertEqual(actual_result.result_set, expected)
         # Verify that the Filter op appears directly above the Apply operation in the ExecutionPlan.
-        plan = redis_graph.execution_plan(query)
-        self.env.assertTrue(re.search('Filter\s+Apply', plan))
+        plan = redis_graph.explain(query)
+        filterOp = locate_operation(plan.structured_plan, "Filter")
+        self.env.assertTrue(filterOp and len(filterOp.children) == 1)
+        applyOp = locate_operation(filterOp, "Apply")
+        self.env.assertTrue(applyOp)
 
         # Verify that filters on projected aliases do not get placed before the projection op.
         query = """UNWIND [1] AS a WITH a AS b, 'projected' AS a WHERE a = 1 RETURN a"""
-        plan = redis_graph.execution_plan(query)
+        plan = redis_graph.explain(query)
         actual_result = redis_graph.query(query)
         expected = [] # No results should be returned
         self.env.assertEqual(actual_result.result_set, expected)
-        self.env.assertTrue(re.search('Filter\s+Project', plan))
+        filterOp = locate_operation(plan.structured_plan, "Filter")
+        self.env.assertTrue(filterOp and len(filterOp.children) == 1)
+        projectOp = locate_operation(filterOp, "Project")
+        self.env.assertTrue(projectOp)
 
         query = """UNWIND [1] AS a WITH a AS b, 'projected' AS a WHERE a = 'projected' RETURN a"""
-        plan = redis_graph.execution_plan(query)
+        plan = redis_graph.explain(query)
         actual_result = redis_graph.query(query)
         expected = [['projected']] # The projected string should be returned
-        self.env.assertTrue(re.search('Filter\s+Project', plan))
+        # self.env.assertTrue(re.search('Filter | a = \'projected\'\s+Project', plan))
+        filterOp = locate_operation(plan.structured_plan, "Filter")
+        self.env.assertTrue(filterOp and len(filterOp.children) == 1)
+        projectOp = locate_operation(filterOp, "Project")
+        self.env.assertTrue(projectOp)
 
     def test11_valid_order_by_aliases(self):
         # Verify that ORDER BY aliases match previously defined references