mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 10:48:20 +00:00
fix(cypher): walk operator chains in the early WHERE pass in a loop
eval_expr_partial, the three-valued evaluator that execute_single runs on every seed row before relationship expansion, recursed into the right child of every AND / OR / XOR node. With operator chains linked along the right child, that is one stack frame per operand: the 50,000-operand OR and AND chain tests overflowed the 256 KiB query worker stack in this early pass, before the already iterative eval_expr ever ran. Walk a chain the way eval_expr does: recurse into NOT and into each operand, and follow the links in a loop. The operands are read in the same order and with the same short-circuits as the nested form (AND stops at the first definite false, OR at the first definite true, XOR reads every operand), and the running value is combined three-valued, so an UNKNOWN operand anywhere in the chain keeps the seed for the full evaluation after expansion. A new test runs three-operand OR / XOR / AND chains whose UNKNOWN operand sits on a chain link and not only on the chain's head. A loop that lets a later operand overwrite the UNKNOWN prunes the seed and fails it. eval_expr_partial moves into the whitelist's chain-walker group with eval_expr and cypher_expr_selectivity: it still recurses, but only as deep as the parse-depth cap allows. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
1 parent
415abe9c2d
commit
3c951326c2
3 files changed
+88
-25
No files matched your search
+47
-21
@@ -2886,6 +2886,31 @@ static cyp_partial_t eval_condition_partial(const cbm_condition_t *c, binding_t
|
||||
return eval_condition(c, b) ? CYP_PARTIAL_TRUE : CYP_PARTIAL_FALSE;
|
||||
}
|
||||
|
||||
static cyp_partial_t partial_xor(cyp_partial_t l, cyp_partial_t r) {
|
||||
if (l == CYP_PARTIAL_UNKNOWN || r == CYP_PARTIAL_UNKNOWN) {
|
||||
return CYP_PARTIAL_UNKNOWN;
|
||||
}
|
||||
return l != r ? CYP_PARTIAL_TRUE : CYP_PARTIAL_FALSE;
|
||||
}
|
||||
|
||||
/* Combine the running value of an AND / OR / XOR chain with its next operand. */
|
||||
static cyp_partial_t partial_combine(cbm_expr_type_t type, cyp_partial_t acc, cyp_partial_t v) {
|
||||
if (type == EXPR_AND) {
|
||||
return partial_and(acc, v);
|
||||
}
|
||||
if (type == EXPR_OR) {
|
||||
return partial_or(acc, v);
|
||||
}
|
||||
return partial_xor(acc, v);
|
||||
}
|
||||
|
||||
/* Same walk as eval_expr: recurses into NOT and into each operand, and follows
|
||||
* a chain of one operator (linked along the right child, see expr_chain_link)
|
||||
* in a loop, so a chain's length never becomes stack depth. This runs on every
|
||||
* seed row of execute_single, so it sees every WHERE chain the parser accepts.
|
||||
* The operands are read in the order of the nested form `a OP (b OP (c ...))`,
|
||||
* with its short-circuits: AND stops at the first definite false, OR at the
|
||||
* first definite true, and XOR reads every operand. */
|
||||
static cyp_partial_t eval_expr_partial(const cbm_expr_t *e, // NOLINT(misc-no-recursion)
|
||||
binding_t *b) {
|
||||
if (!e) {
|
||||
@@ -2894,32 +2919,33 @@ static cyp_partial_t eval_expr_partial(const cbm_expr_t *e, // NOLINT(misc-no-re
|
||||
if (e->type == EXPR_CONDITION) {
|
||||
return eval_condition_partial(&e->cond, b);
|
||||
}
|
||||
cyp_partial_t left = eval_expr_partial(e->left, b);
|
||||
if (e->type == EXPR_NOT) {
|
||||
if (left == CYP_PARTIAL_UNKNOWN) {
|
||||
cyp_partial_t child = eval_expr_partial(e->left, b);
|
||||
if (child == CYP_PARTIAL_UNKNOWN) {
|
||||
return CYP_PARTIAL_UNKNOWN;
|
||||
}
|
||||
return left == CYP_PARTIAL_TRUE ? CYP_PARTIAL_FALSE : CYP_PARTIAL_TRUE;
|
||||
return child == CYP_PARTIAL_TRUE ? CYP_PARTIAL_FALSE : CYP_PARTIAL_TRUE;
|
||||
}
|
||||
/* Same short-circuits as eval_expr: a definite left decides AND / OR. */
|
||||
if (e->type == EXPR_AND && left == CYP_PARTIAL_FALSE) {
|
||||
return CYP_PARTIAL_FALSE;
|
||||
/* AND / OR / XOR. A right child that is not a link of the same operator is
|
||||
* the chain's last operand; one that is a parenthesised group of the same
|
||||
* operator is read as more links, which the operator's associativity
|
||||
* (three-valued as well) makes the same value. */
|
||||
const cbm_expr_type_t type = e->type;
|
||||
cyp_partial_t acc = eval_expr_partial(e->left, b);
|
||||
const cbm_expr_t *rest = e->right;
|
||||
for (;;) {
|
||||
if ((type == EXPR_AND && acc == CYP_PARTIAL_FALSE) ||
|
||||
(type == EXPR_OR && acc == CYP_PARTIAL_TRUE)) {
|
||||
return acc;
|
||||
}
|
||||
bool linked = rest && rest->type == type;
|
||||
cyp_partial_t v = eval_expr_partial(linked ? rest->left : rest, b);
|
||||
acc = partial_combine(type, acc, v);
|
||||
if (!linked) {
|
||||
return acc;
|
||||
}
|
||||
rest = rest->right;
|
||||
}
|
||||
if (e->type == EXPR_OR && left == CYP_PARTIAL_TRUE) {
|
||||
return CYP_PARTIAL_TRUE;
|
||||
}
|
||||
cyp_partial_t right = eval_expr_partial(e->right, b);
|
||||
if (e->type == EXPR_AND) {
|
||||
return partial_and(left, right);
|
||||
}
|
||||
if (e->type == EXPR_OR) {
|
||||
return partial_or(left, right);
|
||||
}
|
||||
/* EXPR_XOR */
|
||||
if (left == CYP_PARTIAL_UNKNOWN || right == CYP_PARTIAL_UNKNOWN) {
|
||||
return CYP_PARTIAL_UNKNOWN;
|
||||
}
|
||||
return left != right ? CYP_PARTIAL_TRUE : CYP_PARTIAL_FALSE;
|
||||
}
|
||||
|
||||
static cyp_partial_t eval_where_partial(const cbm_where_clause_t *w, binding_t *b) {
|
||||
|
||||
@@ -11,10 +11,7 @@
|
||||
*
|
||||
* Cypher expression walkers (bounded by the same parse depth; each follows
|
||||
* an operator chain in a loop and recurses only into the operands):
|
||||
* - eval_expr, cypher_expr_selectivity
|
||||
*
|
||||
* Cypher partial evaluator (bounded by WHERE clause depth ~5):
|
||||
* - eval_expr_partial
|
||||
* - eval_expr, eval_expr_partial, cypher_expr_selectivity
|
||||
*
|
||||
* Glob pattern matcher (bounded by pattern nesting ~3):
|
||||
* - glob_match, glob_match_star, glob_match_doublestar
|
||||
|
||||
@@ -5389,6 +5389,45 @@ static int where_forms_agree(cbm_store_t *s, const char *const *forms, int form_
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* The early WHERE pass (execute_single) binds only the first MATCH alias, so
|
||||
* an operand on the second alias is UNKNOWN there and must neither decide nor
|
||||
* be dropped from a chain: three and more operands put it on a chain link and
|
||||
* not only on the chain's head. The rows are the full evaluation's; an early
|
||||
* pass that read UNKNOWN as false, or let a later operand overwrite it, would
|
||||
* prune the HandleOrder seed and lose them. */
|
||||
TEST(cypher_chain_with_unbound_alias_keeps_seed) {
|
||||
cbm_store_t *s = setup_cypher_store();
|
||||
|
||||
/* OR, UNKNOWN first: only HandleOrder -> LogError has a target without "Order". */
|
||||
static const char *const log_error[] = {"LogError"};
|
||||
ASSERT_EQ(assert_name_set(s,
|
||||
"MATCH (a:Function)-[:CALLS]->(b:Function) WHERE NOT b.name CONTAINS "
|
||||
"\"Order\" OR a.name = \"NoSuchA\" OR a.name = \"NoSuchB\" "
|
||||
"RETURN b.name",
|
||||
log_error, 1),
|
||||
0);
|
||||
|
||||
/* XOR, UNKNOWN in the middle: F ^ T ^ F holds for HandleOrder -> LogError only. */
|
||||
ASSERT_EQ(assert_name_set(s,
|
||||
"MATCH (a:Function)-[:CALLS]->(b:Function) WHERE a.name = "
|
||||
"\"NoSuchA\" XOR b.name = \"LogError\" XOR a.name = \"NoSuchB\" "
|
||||
"RETURN b.name",
|
||||
log_error, 1),
|
||||
0);
|
||||
|
||||
/* AND, UNKNOWN first and a definite operand last: HandleOrder's two calls. */
|
||||
static const char *const handle_order_calls[] = {"LogError", "ValidateOrder"};
|
||||
ASSERT_EQ(assert_name_set(s,
|
||||
"MATCH (a:Function)-[:CALLS]->(b:Function) WHERE b.name <> "
|
||||
"\"SubmitOrder\" AND a.name <> \"NoSuchA\" AND a.name = "
|
||||
"\"HandleOrder\" RETURN b.name",
|
||||
handle_order_calls, 2),
|
||||
0);
|
||||
|
||||
cbm_store_close(s);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* A chain and the same expression written out with parentheses, in both
|
||||
* associations, must agree row for row. The operands include a property the
|
||||
* `main` node does not have: file_path reads as the empty string there, so
|
||||
@@ -5781,6 +5820,7 @@ SUITE(cypher) {
|
||||
RUN_TEST(cypher_long_or_chain_evaluates);
|
||||
RUN_TEST(cypher_long_and_chain_evaluates);
|
||||
RUN_TEST(cypher_mixed_chain_matches_nested_form);
|
||||
RUN_TEST(cypher_chain_with_unbound_alias_keeps_seed);
|
||||
RUN_TEST(cypher_many_union_branches);
|
||||
RUN_TEST(cypher_fifty_union_branches);
|
||||
}
|
||||
Reference in new issue
Block a user