Merge branch 'fix/479-optional-match-join-planning-family'

Post-WITH OPTIONAL MATCH predicate handling: pure-anchor WHERE
conjuncts now move into the LEFT JOIN ON (safe for any LEFT JOIN,
previously only optional-alias conjuncts were moved); and a
general-purpose predicate walker bug that silently dropped IS NULL
operators and corrupted NOT(...) OR ... into NOT(...) AND ... is fixed
at its root cause (broader than originally filed — reproduces on plain
non-WITH cross-alias predicates too).

Three related issues (#478, #461, #479) are deferred with "KNOWN
BROKEN" characterization tests rather than risking another naive fix
that's worse than the original bug (this exact family already burned
that once, per #479's own ground-truth experiment) — they need
structural planner/join-extraction work, precisely characterized for
a future slice.

Fixes #472
Fixes #473

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: genezhang

src/query_planner/analyzer/filter_tagging.rs

@@ -1707,18 +1707,28 @@ impl FilterTagging {
                     let single_table =
                         Self::get_table_alias_if_single_table_condition(&op_app.operands[0], false);
 
-                    if single_table.is_none() {
-                        // Cross-table condition under NOT (e.g., NOT (p.id = friend.id))
-                        // Keep it intact - don't recurse into operands
-                        // Just extract property projections for column selection
+                    // #473: a single-table NOT nested inside an OR (`in_or`) must NOT be
+                    // extracted as its own standalone filter — doing so silently splits the
+                    // OR into two independently-placed conjuncts (effectively an AND),
+                    // changing the query's boolean semantics (e.g.
+                    // `NOT(o.amount > 5) OR c.id > 101` was rendered as if it were
+                    // `NOT(o.amount > 5) AND c.id > 101`, dropping matches the OR should
+                    // have kept). Only extract when NOT is single-table AND not nested in
+                    // an OR — mirrors the general single-table extraction guard below
+                    // (`!new_in_or && ... && is_single_table_condition`).
+                    if single_table.is_none() || in_or {
+                        // Cross-table condition under NOT (e.g., NOT (p.id = friend.id)),
+                        // or a single-table NOT that lives inside an OR: keep it intact -
+                        // don't recurse into operands. Just extract property projections
+                        // for column selection.
                         let mut temp_props = Vec::new();
                         Self::collect_property_accesses(&op_app.operands[0], &mut temp_props);
                         extracted_projections.extend(temp_props);
 
                         // Return the entire NOT expression as-is for global WHERE clause
                         return Some(LogicalExpr::OperatorApplicationExp(op_app));
                     } else {
-                        // Single-table NOT (e.g., NOT p.active)
+                        // Single-table NOT (e.g., NOT p.active), NOT inside an OR
                         // Extract it as a complete filter to the table
                         // Collect projections from the operand
                         let mut temp_props = Vec::new();
@@ -1843,8 +1853,19 @@ impl FilterTagging {
                     extracted_projections.append(&mut temp_prop_acc);
                 }
 
-                // If after processing there is only one operand left and it is not unary then collapse the operator application.
-                if op_app.operands.len() == 1 && op_app.operator != Operator::Not {
+                // If after processing there is only one operand left, collapse the operator
+                // application — but ONLY for the variadic boolean combinators (And/Or) whose
+                // operand count can genuinely shrink from extraction (e.g. `a AND b` becomes
+                // just `b` once `a` is extracted as its own filter). #473: this must NOT fire
+                // for operators that are inherently single-operand (Not, IsNull, IsNotNull,
+                // Distinct) — those always have exactly one operand from the start, and
+                // "collapsing" them silently drops the operator itself (e.g.
+                // `o.amount IS NULL` was rendered as bare `o.amount`, evaluating a Float
+                // column as a boolean). The old `operator != Operator::Not` check excluded
+                // only Not, missing IsNull/IsNotNull/Distinct.
+                if op_app.operands.len() == 1
+                    && matches!(op_app.operator, Operator::And | Operator::Or)
+                {
                     log::warn!(
                         "process_expr: Collapsing operator {:?} with single operand: {:?}",
                         op_app.operator,

src/render_plan/plan_builder_utils.rs

@@ -13076,53 +13076,56 @@ pub(crate) fn build_chained_with_match_cte_plan(
                         );
                     }
 
-                    // #462 GAP 1: cross-alias / OR conjuncts were routed to the
-                    // outer WHERE (`render_plan.filters`) by
-                    // `collect_graphrel_predicates` — wrong for OPTIONAL MATCH, as
-                    // the outer WHERE drops the NULL-extended no-match anchor rows.
-                    // Move every already-resolved filter conjunct that references
-                    // the optional node alias into the LEFT JOIN's ON condition
-                    // (anchor `c` references were already resolved to CTE columns,
-                    // e.g. `c.p1_c_customer_id`, when the filter was rendered).
-                    // Pure-anchor conjuncts stay in the outer WHERE.
+                    // #462/#472: every conjunct in `render_plan.filters` at this
+                    // point belongs EXCLUSIVELY to this post-WITH OPTIONAL MATCH's
+                    // own WHERE (the tight `post_with_optional_restructure` guard
+                    // above ensures a single-branch optional pattern; any WHERE
+                    // attached to the WITH projection itself was already folded
+                    // into the CTE body and is not visible here). `
+                    // collect_graphrel_predicates` routed the whole thing to the
+                    // outer WHERE — wrong for OPTIONAL MATCH, since the outer WHERE
+                    // drops the NULL-extended no-match anchor rows. Move EVERY
+                    // conjunct into the LEFT JOIN's ON condition, including
+                    // pure-anchor ones (#472): for a LEFT JOIN a false ON condition
+                    // just NULL-extends the row rather than dropping it, so folding
+                    // the whole predicate into ON is always safe and never changes
+                    // which rows are kept vs. dropped when the FROM side is the
+                    // anchor CTE. (Anchor `c` references were already resolved to
+                    // CTE columns, e.g. `c.p1_c_customer_id`, when the filter was
+                    // rendered.) Nothing is left behind in the outer WHERE for this
+                    // segment.
                     let mut extra_on_conditions: Vec<OperatorApplication> = Vec::new();
                     if let Some(filter_expr) = render_plan.filters.0.take() {
-                        let mut kept: Vec<RenderExpr> = Vec::new();
                         for conj in split_render_and_conjuncts(filter_expr) {
-                            if super::expression_utils::references_alias(
-                                &conj,
-                                &optional_from_alias,
-                            ) {
-                                match conj {
-                                    RenderExpr::OperatorApplicationExp(op) => {
-                                        extra_on_conditions.push(op);
-                                    }
-                                    // A boolean conjunct that is not an operator
-                                    // application (e.g. a bare scalar-fn predicate)
-                                    // cannot be expressed as a joining_on
-                                    // `OperatorApplication`. Rather than silently
-                                    // leave it in the outer WHERE (wrong semantics)
-                                    // or drop it, refuse with a clean error (#462).
-                                    other => {
-                                        return Err(RenderBuildError::InvalidRenderPlan(format!(
-                                            "post-WITH OPTIONAL MATCH WHERE conjunct referencing \
-                                             optional alias '{}' is not an operator application and \
-                                             cannot be moved into the LEFT JOIN ON condition; \
-                                             refusing to place it in the outer WHERE (would drop \
-                                             NULL-extended rows): {:?}",
-                                            optional_from_alias, other
-                                        )));
-                                    }
+                            match conj {
+                                RenderExpr::OperatorApplicationExp(op) => {
+                                    extra_on_conditions.push(op);
+                                }
+                                // A boolean conjunct that is not an operator
+                                // application (e.g. a bare scalar-fn predicate)
+                                // cannot be expressed as a joining_on
+                                // `OperatorApplication`. Rather than silently leave
+                                // it in the outer WHERE (wrong semantics) or drop
+                                // it, refuse with a clean error (#462/#472).
+                                other => {
+                                    return Err(RenderBuildError::InvalidRenderPlan(format!(
+                                        "post-WITH OPTIONAL MATCH WHERE conjunct is not an \
+                                         operator application and cannot be moved into the \
+                                         LEFT JOIN ON condition for alias '{}'; refusing to \
+                                         place it in the outer WHERE (would drop NULL-extended \
+                                         rows): {:?}",
+                                        optional_from_alias, other
+                                    )));
                                 }
-                            } else {
-                                kept.push(conj);
                             }
                         }
-                        render_plan.filters.0 = combine_render_exprs_with_and(kept);
+                        // No `kept` remainder: the whole post-OPTIONAL WHERE moves
+                        // into the ON condition, so `render_plan.filters` stays
+                        // cleared (no outer WHERE for this segment).
                     }
                     if !extra_on_conditions.is_empty() {
                         log::info!(
-                            "🔧 build_chained_with_match_cte_plan: #462 moved {} cross-alias/OR WHERE conjunct(s) into LEFT JOIN ON for alias '{}'",
+                            "🔧 build_chained_with_match_cte_plan: #462/#472 moved {} WHERE conjunct(s) (incl. pure-anchor) into LEFT JOIN ON for alias '{}'",
                             extra_on_conditions.len(),
                             optional_from_alias
                         );