Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added params support to id seek #1164

Merged
merged 5 commits into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/arithmetic/arithmetic_expression.c
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,18 @@ AR_ExpNode *AR_EXP_NewParameterOperandNode(const char *param_name) {
* e.g. MINUS(X) where X is a constant number will be reduced to
* a single node with the value -X
* PLUS(MINUS(A), B) will be reduced to a single constant: B-A. */
bool AR_EXP_ReduceToScalar(AR_ExpNode *root) {
bool AR_EXP_ReduceToScalar(AR_ExpNode *root, bool reduce_params, SIValue *val) {
if(val != NULL) *val = SI_NullVal();
if(root->type == AR_EXP_OPERAND) {
if(root->operand.type == AR_EXP_CONSTANT) {
// In runtime, parameters are set so they can be evaluated
if(reduce_params && AR_EXP_IsParameter(root)) {
SIValue v = AR_EXP_Evaluate(root, NULL);
if(val != NULL) *val = v;
return true;
}
if(AR_EXP_IsConstant(root)) {
// Root is already a constant
if(val != NULL) *val = root->operand.constant;
return true;
}
// Root is variadic, no way to reduce.
Expand All @@ -189,7 +197,7 @@ bool AR_EXP_ReduceToScalar(AR_ExpNode *root) {
* if so we'll be able to reduce root. */
bool reduce_children = true;
for(int i = 0; i < root->op.child_count; i++) {
if(!AR_EXP_ReduceToScalar(root->op.children[i])) {
if(!AR_EXP_ReduceToScalar(root->op.children[i], reduce_params, NULL)) {
// Root reduce is not possible, but continue to reduce every reducable child.
reduce_children = false;
}
Expand All @@ -204,6 +212,7 @@ bool AR_EXP_ReduceToScalar(AR_ExpNode *root) {

// Evaluate function.
SIValue v = AR_EXP_Evaluate(root, NULL);
if(val != NULL) *val = v;
if(SIValue_IsNull(v)) return false;

// Reduce.
Expand Down Expand Up @@ -444,7 +453,7 @@ SIValue AR_EXP_Evaluate(AR_ExpNode *root, const Record r) {
}
// At least one param node was encountered during evaluation, tree should be param node free.
// Try reducing the tree.
if(res == EVAL_FOUND_PARAM) AR_EXP_ReduceToScalar(root);
if(res == EVAL_FOUND_PARAM) AR_EXP_ReduceToScalar(root, true, NULL);
return result;
}

Expand Down
7 changes: 5 additions & 2 deletions src/arithmetic/arithmetic_expression.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,11 @@ AR_ExpNode *AR_EXP_NewParameterOperandNode(const char *param_name);
/* Returns if the operation is distinct aggregation */
bool AR_EXP_PerformDistinct(AR_ExpNode *op);

/* Compact tree by evaluating all contained functions that can be resolved right now. */
bool AR_EXP_ReduceToScalar(AR_ExpNode *root);
/* Compact tree by evaluating all contained functions that can be resolved right now.
* The function returns true if it managed to compact the expression.
* The reduce_params flag indicates if parameters should be evaluated.
* The val pointer is out-by-ref returned computation. */
bool AR_EXP_ReduceToScalar(AR_ExpNode *root, bool reduce_params, SIValue *val);

/* Evaluate arithmetic expression tree. */
SIValue AR_EXP_Evaluate(AR_ExpNode *root, const Record r);
Expand Down
2 changes: 1 addition & 1 deletion src/ast/ast_build_ar_exp.c
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ static AR_ExpNode *_AR_EXP_FromExpression(const cypher_astnode_t *expr) {

AR_ExpNode *AR_EXP_FromExpression(const cypher_astnode_t *expr) {
AR_ExpNode *root = _AR_EXP_FromExpression(expr);
AR_EXP_ReduceToScalar(root);
AR_EXP_ReduceToScalar(root, false, NULL);

/* Make sure expression doesn't contains nested aggregation functions
* count(max(n.v)) */
Expand Down
16 changes: 9 additions & 7 deletions src/execution_plan/optimizations/seek_by_id.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ static bool _idFilter(FT_FilterNode *f, AST_Operator *rel, EntityID *id, bool *r
if(f->pred.op == OP_NEQUAL) return false;

AR_OpNode *op;
AR_OperandNode *operand;
AR_ExpNode *expr;
AR_ExpNode *lhs = f->pred.lhs;
AR_ExpNode *rhs = f->pred.rhs;
*rel = f->pred.op;
Expand All @@ -29,20 +29,22 @@ static bool _idFilter(FT_FilterNode *f, AST_Operator *rel, EntityID *id, bool *r
* const compare ID(N) */
if(lhs->type == AR_EXP_OPERAND && rhs->type == AR_EXP_OP) {
op = &rhs->op;
operand = &lhs->operand;
expr = lhs;
*reverse = true;
} else if(lhs->type == AR_EXP_OP && rhs->type == AR_EXP_OPERAND) {
op = &lhs->op;
operand = &rhs->operand;
expr = rhs;
*reverse = false;
} else {
return false;
}

// Make sure ID is compared to a constant.
if(operand->type != AR_EXP_CONSTANT) return false;
if(SI_TYPE(operand->constant) != T_INT64) return false;
*id = SI_GET_NUMERIC(operand->constant);
// Make sure ID is compared to a constant int64.
SIValue val;
bool reduced = AR_EXP_ReduceToScalar(expr, true, &val);
if(!reduced) return false;
if(SI_TYPE(val) != T_INT64) return false;
*id = SI_GET_NUMERIC(val);

// Make sure applied function is ID.
if(strcasecmp(op->func_name, "id")) return false;
Expand Down
26 changes: 10 additions & 16 deletions src/execution_plan/optimizations/utilize_indices.c
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,10 @@ static bool _validateInExpression(AR_ExpNode *exp) {
assert(exp->op.child_count == 2);

AR_ExpNode *list = exp->op.children[1];
// In case the array is a parameter such as "WHERE x IN $arr", evaluate the parameter for in-place replacement.
if(AR_EXP_IsParameter(list)) AR_EXP_Evaluate(list, NULL);
if(list->operand.type != AR_EXP_CONSTANT || list->operand.constant.type != T_ARRAY) return false;
SIValue listValue = SI_NullVal();
AR_EXP_ReduceToScalar(list, true, &listValue);
if(SI_TYPE(listValue) != T_ARRAY) return false;

SIValue listValue = list->operand.constant;
uint listLen = SIArray_Length(listValue);
for(uint i = 0; i < listLen; i++) {
SIValue v = SIArray_Get(listValue, i);
Expand All @@ -242,20 +241,15 @@ bool _simple_predicates(const FT_FilterNode *filter) {
case FT_N_PRED:
if(filter->pred.rhs->type == AR_EXP_OPERAND &&
filter->pred.lhs->type == AR_EXP_OPERAND) {
// In case of parameters, evalation will transform them into constants (in-place replacement).
if(AR_EXP_IsParameter(filter->pred.lhs)) AR_EXP_Evaluate(filter->pred.lhs, NULL);
if(AR_EXP_IsParameter(filter->pred.rhs)) AR_EXP_Evaluate(filter->pred.rhs, NULL);
if((filter->pred.lhs->operand.type == AR_EXP_CONSTANT ||
filter->pred.rhs->operand.type == AR_EXP_CONSTANT) &&
(filter->pred.lhs->operand.type == AR_EXP_VARIADIC ||
filter->pred.lhs->operand.type == AR_EXP_VARIADIC)) {

SIValue v_lhs = SI_NullVal();
SIValue v_rhs = SI_NullVal();
bool lhs_scalar = AR_EXP_ReduceToScalar(filter->pred.lhs, true, &v_lhs);
bool rhs_scalar = AR_EXP_ReduceToScalar(filter->pred.rhs, true, &v_rhs);
// Predicate should be in the form of variable=scalar or scalar=variadic
if((lhs_scalar && !rhs_scalar) || (!lhs_scalar && rhs_scalar)) {
// Validate constant type.
SIValue c = SI_NullVal();
if(filter->pred.lhs->operand.type == AR_EXP_CONSTANT) c = filter->pred.lhs->operand.constant;
if(filter->pred.rhs->operand.type == AR_EXP_CONSTANT) c = filter->pred.rhs->operand.constant;
SIValue c = lhs_scalar ? v_lhs : v_rhs;
SIType t = SI_TYPE(c);

res = (t & (SI_NUMERIC | T_STRING | T_BOOL));
}
}
Expand Down
12 changes: 12 additions & 0 deletions tests/flow/test_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,15 @@ def test_missing_parameter(self):
except:
# Expecting an error.
pass

def test_id_scan(self):
redis_graph.query("CREATE ({val:1})")
expected_results=[[1]]
params = {'id' : 0}
query = "MATCH (n) WHERE id(n)=$id return n.val"
query_info = QueryInfo(query = query, description="Test id scan with params", expected_result = expected_results)
self._assert_resultset_equals_expected(redis_graph.query(query, params), query_info)
query = redis_graph.build_params_header(params) + query
plan = redis_graph.execution_plan(query)
self.env.assertIn('NodeByIdSeek', plan)