Skip to content

Add algebraic simplifications to constant_folding #1208

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Nov 2, 2021
Merged

Add algebraic simplifications to constant_folding #1208

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c75922f
Add Or branch to constant folding
matthewmturner Oct 31, 2021
ff060ea
Add test for Or
matthewmturner Oct 31, 2021
e04a8dd
Simplify or match
matthewmturner Nov 1, 2021
fc715de
Add And operator
matthewmturner Nov 1, 2021
c240e83
Updated Or and And logic
matthewmturner Nov 1, 2021
056508d
Updated Or and And to handle nulls and added tests
matthewmturner Nov 2, 2021
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
207 changes: 207 additions & 0 deletions datafusion/src/optimizer/constant_folding.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,104 @@ impl<'a> ExprRewriter for Simplifier<'a> {
right,
},
},
Operator::Or => match (left.as_ref(), right.as_ref()) {
(Expr::Literal(ScalarValue::Boolean(b)), _)
if self.is_boolean_type(&right) =>
Copy link
Contributor Author

@matthewmturner matthewmturner Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alamb to help me understand, can you provide color on whats an example of an Expr that isnt a Boolean but that has a boolean type?

Copy link
Contributor Author

@matthewmturner matthewmturner Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, i get it now.

Copy link
Contributor

@alamb alamb Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One example is if someone writes col_a OR true and col_a has type of int64 or something -- I don't know how far such an expression will get (it will likely error out when trying to to actually run the PhysicalExpr)

{
match b {
Some(true) => Expr::Literal(ScalarValue::Boolean(Some(true))),
Copy link
Contributor

@alamb alamb Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so cool!

Copy link
Contributor

@alamb alamb Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can match null as well -- aka ScalarValue::Boolean(None)

because null OR true == true

alamb=# select null or true;
 ?column? 
----------
 t
(1 row)

Some(false) => match *right {
Expr::Literal(ScalarValue::Boolean(None)) => {
Expr::Literal(ScalarValue::Boolean(None))
}
_ => *right,
},
None => match *right {
Expr::Literal(ScalarValue::Boolean(Some(true))) => {
Expr::Literal(ScalarValue::Boolean(Some(true)))
}
Expr::Literal(ScalarValue::Boolean(Some(false))) => {
Expr::Literal(ScalarValue::Boolean(None))
}
_ => *right,
},
}
}
(_, Expr::Literal(ScalarValue::Boolean(b)))
if self.is_boolean_type(&left) =>
{
match b {
Some(true) => Expr::Literal(ScalarValue::Boolean(Some(true))),
Copy link
Contributor

@alamb alamb Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also add the rules that null OR false --> null

alamb=# select null or false;
 ?column? 
----------
 
(1 row)

Some(false) => match *left {
Expr::Literal(ScalarValue::Boolean(None)) => {
Expr::Literal(ScalarValue::Boolean(None))
}
_ => *left,
},
None => match *left {
Expr::Literal(ScalarValue::Boolean(Some(true))) => {
Expr::Literal(ScalarValue::Boolean(Some(true)))
}
Expr::Literal(ScalarValue::Boolean(Some(false))) => {
Expr::Literal(ScalarValue::Boolean(None))
}
_ => *left,
},
}
}
_ => Expr::BinaryExpr {
left,
op: Operator::Or,
right,
},
},
Operator::And => match (left.as_ref(), right.as_ref()) {
Copy link
Contributor

@alamb alamb Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rules for null and AND are NULL AND true --> NULL, and NULL AND false --> false

alamb=# select null and true;
 ?column? 
----------
 
(1 row)

alamb=# select null and false;
 ?column? 
----------
 f
(1 row)

(Expr::Literal(ScalarValue::Boolean(b)), _)
if self.is_boolean_type(&right) =>
{
// match b {
// Some(false) => {
// Expr::Literal(ScalarValue::Boolean(Some(false)))
// }
// _ => *right,
// }
match b {
Some(true) => match *right {
Expr::Literal(ScalarValue::Boolean(None)) => {
Expr::Literal(ScalarValue::Boolean(None))
}
_ => *right,
},
Some(false) => {
Expr::Literal(ScalarValue::Boolean(Some(false)))
}
None => match *right {
Expr::Literal(ScalarValue::Boolean(Some(true))) => {
Expr::Literal(ScalarValue::Boolean(None))
}
Expr::Literal(ScalarValue::Boolean(Some(false))) => {
Expr::Literal(ScalarValue::Boolean(Some(false)))
}
_ => *right,
},
}
}
(_, Expr::Literal(ScalarValue::Boolean(b)))
if self.is_boolean_type(&left) =>
{
match b {
Some(false) => {
Expr::Literal(ScalarValue::Boolean(Some(false)))
}
_ => *left,
}
}
_ => Expr::BinaryExpr {
left,
op: Operator::And,
right,
},
},
_ => Expr::BinaryExpr { left, op, right },
},
// Not(Not(expr)) --> expr
Expand Down Expand Up @@ -811,4 +909,113 @@ mod tests {

assert_eq!(expected, actual);
}
#[test]
fn optimize_expr_bool_or() -> Result<()> {
let schema = expr_test_schema();
let mut rewriter = Simplifier {
schemas: vec![&schema],
};

// col || true is always true
assert_eq!(
(col("c2").or(Expr::Literal(ScalarValue::Boolean(Some(true)))))
.rewrite(&mut rewriter)?,
lit(ScalarValue::Boolean(Some(true))),
);

// col || false is always col
assert_eq!(
(col("c2").or(Expr::Literal(ScalarValue::Boolean(Some(false)))))
.rewrite(&mut rewriter)?,
col("c2"),
);

// true || null is always true
assert_eq!(
(Expr::Literal(ScalarValue::Boolean(Some(true)))
.or(Expr::Literal(ScalarValue::Boolean(None))))
.rewrite(&mut rewriter)?,
lit(ScalarValue::Boolean(Some(true))),
);

// null || true is always true
assert_eq!(
(Expr::Literal(ScalarValue::Boolean(None))
.or(Expr::Literal(ScalarValue::Boolean(Some(true)))))
.rewrite(&mut rewriter)?,
lit(ScalarValue::Boolean(Some(true))),
);

// false || null is always null
assert_eq!(
(Expr::Literal(ScalarValue::Boolean(Some(false)))
.or(Expr::Literal(ScalarValue::Boolean(None))))
.rewrite(&mut rewriter)?,
lit(ScalarValue::Boolean(None)),
);

// null || false is always null
assert_eq!(
(Expr::Literal(ScalarValue::Boolean(None))
.or(Expr::Literal(ScalarValue::Boolean(Some(false)))))
.rewrite(&mut rewriter)?,
lit(ScalarValue::Boolean(None)),
);

Ok(())
}
#[test]
fn optimize_expr_bool_and() -> Result<()> {
let schema = expr_test_schema();
let mut rewriter = Simplifier {
schemas: vec![&schema],
};

// col & true is always col
assert_eq!(
(col("c2").and(Expr::Literal(ScalarValue::Boolean(Some(true)))))
.rewrite(&mut rewriter)?,
col("c2"),
);
// col & false is always false
assert_eq!(
(col("c2").and(Expr::Literal(ScalarValue::Boolean(Some(false)))))
.rewrite(&mut rewriter)?,
lit(ScalarValue::Boolean(Some(false))),
);

// true && null is always null
assert_eq!(
(Expr::Literal(ScalarValue::Boolean(Some(true)))
.and(Expr::Literal(ScalarValue::Boolean(None))))
.rewrite(&mut rewriter)?,
lit(ScalarValue::Boolean(None)),
);

// null && true is always null
assert_eq!(
(Expr::Literal(ScalarValue::Boolean(None))
.and(Expr::Literal(ScalarValue::Boolean(Some(true)))))
.rewrite(&mut rewriter)?,
lit(ScalarValue::Boolean(None)),
);

// false && null is always false
assert_eq!(
(Expr::Literal(ScalarValue::Boolean(Some(false)))
.and(Expr::Literal(ScalarValue::Boolean(None))))
.rewrite(&mut rewriter)?,
lit(ScalarValue::Boolean(Some(false))),
);

// null && false is always false
assert_eq!(
(Expr::Literal(ScalarValue::Boolean(None))
.and(Expr::Literal(ScalarValue::Boolean(Some(false)))))
.rewrite(&mut rewriter)?,
lit(ScalarValue::Boolean(Some(false))),
);

Ok(())
}
}