feat(expr): implement Kleene logic boolean operators#6464
feat(expr): implement Kleene logic boolean operators#6464romeoscript wants to merge 1 commit intovortex-data:developfrom
Conversation
| fn split_inner(expr: &Expression, exprs: &mut Vec<Expression>) { | ||
| match expr.as_opt::<Binary>() { | ||
| Some(operator) if *operator == Operator::And => { | ||
| Some(operator) if *operator == Operator::KleeneAnd => { |
There was a problem hiding this comment.
What is the reason for changing this?
There was a problem hiding this comment.
The reason is that Operator::And now represents Standard logic , but Vortex’s query engine was built assuming Kleene logic
There was a problem hiding this comment.
What about regular and?
| pub fn not_kleene(operand: Expression) -> Expression { | ||
| Not.new_expr(EmptyOptions, vec![operand]) | ||
| } |
There was a problem hiding this comment.
this is not correct?
There was a problem hiding this comment.
aparently this is redundant
There was a problem hiding this comment.
Then why include it?
|
Mind explain the change you made? |
I'm just separating standard boolean logic from Kleene logic to make things explicit. And/Or now follow standard null propagation while KleeneAnd/KleeneOr handle the Kleene-style logic we were using implicitly before. |
| .chain(rhs.stat_falsification(catalog)) | ||
| .reduce(or), | ||
| Operator::Or => Some(and( | ||
| .reduce(or_kleene), |
There was a problem hiding this comment.
Why does or map to or kleene here?
There was a problem hiding this comment.
Yeah, since or() now defaults to strict null handling, we have to use or_kleene() here to keep the current behavior. Plus, Kleene is actually better for stats since it's more aggressive with pruning when there are nulls
2 participants
Does this PR closes an open issue or discussion?
expr::Operator::{And/Or/Not}is actuallyKleene{And/Or/No}#6458 .What changes are included in this PR?