Skip to content

Commit

Permalink
dataflow-expression: Infer Bool as the return type for boolean ops
Browse files Browse the repository at this point in the history
Properly infer Bool as the return type for boolean binary operators,
rather than blindly taking the type of the left-hand side of the
operator. This makes queries like `SELECT x = 1 FROM t` return the
correct type.

Change-Id: I38c8f0f4e22c8bd533dc365f94f72816fe56baf7
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/3588
Tested-by: Buildkite CI
Reviewed-by: Dan Wilbanks <[email protected]>
  • Loading branch information
glittershark committed Nov 1, 2022
1 parent 34b724c commit c21f7ae
Showing 1 changed file with 33 additions and 2 deletions.
35 changes: 33 additions & 2 deletions dataflow-expression/src/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,9 +375,22 @@ impl Expr {
let op = BinaryOperator::from_sql_op(op, dialect);

let left = Box::new(Self::lower(*lhs, dialect, context.clone())?);
// TODO: Add more operators to this match, and maybe consider rhs in some cases too
// TODO: Maybe consider rhs in some cases too
// TODO: What is the correct return type for And and Or?
let ty = match op {
BinaryOperator::QuestionMark => DfType::Bool,
BinaryOperator::Like
| BinaryOperator::NotLike
| BinaryOperator::ILike
| BinaryOperator::NotILike
| BinaryOperator::Equal
| BinaryOperator::NotEqual
| BinaryOperator::Greater
| BinaryOperator::GreaterOrEqual
| BinaryOperator::Less
| BinaryOperator::LessOrEqual
| BinaryOperator::Is
| BinaryOperator::IsNot
| BinaryOperator::QuestionMark => DfType::Bool,
_ => left.ty().clone(),
};
Ok(Self::Op {
Expand Down Expand Up @@ -940,6 +953,24 @@ pub(crate) mod tests {
);
}

#[test]
fn eq_returns_bool() {
let input = parse_expr(ParserDialect::MySQL, "x = 1").unwrap();
let result = Expr::lower(
input,
Dialect::DEFAULT_MYSQL,
resolve_columns(|c| {
if c.name == "x" {
Ok((0, DfType::DEFAULT_TEXT))
} else {
internal!("what's this column?")
}
}),
)
.unwrap();
assert_eq!(*result.ty(), DfType::Bool);
}

#[test]
fn lowered_question_mark_expr_type() {
let input = AstExpr::BinaryOp {
Expand Down

0 comments on commit c21f7ae

Please sign in to comment.