Skip to content

Commit

Permalink
Fix invalid op sequence when partially replacing filters with index s…
Browse files Browse the repository at this point in the history
…cans
  • Loading branch information
jeffreylovitz committed Mar 18, 2020
1 parent f5dac07 commit 379fa40
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
12 changes: 6 additions & 6 deletions src/execution_plan/optimizations/utilize_indices.c
Original file line number Diff line number Diff line change
Expand Up @@ -497,15 +497,15 @@ void reduce_scan_op(ExecutionPlan *plan, NodeByLabelScan *scan) {
RSResultsIterator *iter = RediSearch_GetResultsIterator(root, rs_idx);
// Build the Index Scan.
OpBase *indexOp = NewIndexScanOp(scan->op.plan, scan->g, scan->n, rs_idx, iter);
/* In place, replace the last redundant filter (highest in the op tree) with the new scan op.
* This ensures that the children array of the scan's parent op does not get shuffled,
* avoiding problems with stream-sensitive ops like SemiApply. */
/* Remove the redundant filter op without reordering its parent op's child array
* (avoiding problems with stream-sensitive ops like SemiApply). */
OpBase *last_filter = (OpBase *)array_pop(filters);
filters_count--;
ExecutionPlan_ReplaceOp(plan, last_filter, indexOp);
// Free the redundant filter and scan op.
ExecutionPlan_RemoveOp(plan, last_filter);
OpBase_Free(last_filter);
ExecutionPlan_RemoveOp(plan, (OpBase *)scan);

/* Replace the redundant scan op with the newly-constructed Index Scan. */
ExecutionPlan_ReplaceOp(plan, (OpBase *)scan, indexOp);
OpBase_Free((OpBase *)scan);
}

Expand Down
12 changes: 12 additions & 0 deletions tests/flow/test_index_scans.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,15 @@ def test07_index_scan_and_id(self):
self.env.assertEqual(2, len(query_result.result_set))
expected_result = [[nodes[7]], [nodes[8]]]
self.env.assertEquals(expected_result, query_result.result_set)

# Validate placement of index scans and filter ops when not all filters can be replaced.
def test08_index_scan_multiple_filters(self):
query = "MATCH (p:person) WHERE p.age = 30 AND NOT EXISTS(p.fakeprop) RETURN p.name"
plan = redis_graph.execution_plan(query)
self.env.assertIn('Index Scan', plan)
self.env.assertNotIn('Label Scan', plan)
self.env.assertIn('Filter', plan)

query_result = redis_graph.query(query)
expected_result = ["Lucy Yanfital"]
self.env.assertEquals(query_result.result_set[0], expected_result)

0 comments on commit 379fa40

Please sign in to comment.