Skip to content

Commit

Permalink
Fix nullable expression in WHERE clause
Browse files Browse the repository at this point in the history
  • Loading branch information
cswinter committed Aug 3, 2024
1 parent 8524102 commit 49fdcbe
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/engine/operators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ mod constant_expand;
mod constant_vec;
mod delta_decode;
mod dict_lookup;
mod empty;
mod encode_const;
mod exists;
mod filter;
Expand Down
10 changes: 10 additions & 0 deletions src/engine/operators/vector_operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use super::constant_expand::ConstantExpand;
use super::constant_vec::ConstantVec;
use super::delta_decode::*;
use super::dict_lookup::*;
use super::empty::Empty;
use super::encode_const::*;
use super::exists::Exists;
use super::filter::{Filter, NullableFilter};
Expand Down Expand Up @@ -769,6 +770,15 @@ pub mod operator {
Box::new(ConstantVec { val, output })
}

pub fn empty<'a>(empty: TypedBufferRef) -> Result<BoxedOperator<'a>, QueryError> {
reify_types! {
"empty";
empty: Primitive;
Ok(Box::new(Empty { output: empty }))
}
}


pub fn less_than<'a>(
lhs: TypedBufferRef,
rhs: TypedBufferRef,
Expand Down
2 changes: 2 additions & 0 deletions src/engine/planning/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::engine::{BufferRef, Nullable, QueryPlanner, TypedBufferRef};
pub enum Filter {
#[default]
None,
Null,
U8(BufferRef<u8>),
NullableU8(BufferRef<Nullable<u8>>),
Indices(BufferRef<usize>),
Expand All @@ -16,6 +17,7 @@ impl Filter {
Filter::U8(filter) => planner.filter(plan, filter),
Filter::NullableU8(filter) => planner.nullable_filter(plan, filter),
Filter::Indices(indices) => planner.select(plan, indices),
Filter::Null => planner.empty(plan.tag),
Filter::None => plan,
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/engine/planning/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ impl NormalFormQuery {
let mut filter = match filter_plan.tag {
EncodingType::U8 => Filter::U8(filter_plan.u8()?),
EncodingType::NullableU8 => Filter::NullableU8(filter_plan.nullable_u8()?),
EncodingType::Null => Filter::Null,
_ => Filter::None,
};

Expand Down Expand Up @@ -124,6 +125,7 @@ impl NormalFormQuery {
let filter = planner.nullable_filter(indices, where_true);
Filter::Indices(planner.select(filter, sort_indices).usize()?)
}
Filter::Null => Filter::Null,
Filter::None => Filter::Indices(sort_indices),
Filter::Indices(_) => unreachable!(),
};
Expand Down Expand Up @@ -205,14 +207,15 @@ impl NormalFormQuery {
let mut qp = QueryPlanner::default();

// Filter
let (filter_plan, filter_type) = QueryPlan::compile_expr(
let (filter_plan, _) = QueryPlan::compile_expr(
&self.filter,
Filter::None,
columns,
partition_range.len(),
&mut qp,
)?;
let filter = match filter_type.encoding_type() {
let filter = match filter_plan.tag {
EncodingType::Null => Filter::Null,
EncodingType::U8 => Filter::U8(filter_plan.u8()?),
EncodingType::NullableU8 => Filter::NullableU8(filter_plan.nullable_u8()?),
_ => Filter::None,
Expand Down
37 changes: 37 additions & 0 deletions src/engine/planning/query_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,11 @@ pub enum QueryPlan {
#[output(t = "base=provided")]
constant_vec: TypedBufferRef,
},
/// Outputs an empty vector of the same type as `plan`.
Empty {
#[output(t = "base=provided")]
empty: TypedBufferRef,
},
/// Creates a "null vector" of the specified length and type which encodes the number of null values as a single scalar.
NullVec {
len: usize,
Expand Down Expand Up @@ -951,6 +956,12 @@ fn function2_registry() -> HashMap<Func2Type, Vec<Function2>> {
type_out: Type::unencoded(BasicType::Boolean).mutable(),
encoding_invariance: true,
},
Function2::forward_left_null(BasicType::Float),
Function2::forward_right_null(BasicType::Float),
Function2::forward_left_null(BasicType::Integer),
Function2::forward_right_null(BasicType::Integer),
Function2::forward_left_null(BasicType::Null),
Function2::forward_right_null(BasicType::Null),
],
),
(
Expand Down Expand Up @@ -987,6 +998,12 @@ fn function2_registry() -> HashMap<Func2Type, Vec<Function2>> {
type_out: Type::unencoded(BasicType::Boolean).mutable(),
encoding_invariance: true,
},
Function2::forward_left_null(BasicType::Float),
Function2::forward_right_null(BasicType::Float),
Function2::forward_left_null(BasicType::Integer),
Function2::forward_right_null(BasicType::Integer),
Function2::forward_left_null(BasicType::Null),
Function2::forward_right_null(BasicType::Null),
],
),
(
Expand Down Expand Up @@ -1023,6 +1040,12 @@ fn function2_registry() -> HashMap<Func2Type, Vec<Function2>> {
type_out: Type::unencoded(BasicType::Boolean).mutable(),
encoding_invariance: true,
},
Function2::forward_left_null(BasicType::Float),
Function2::forward_right_null(BasicType::Float),
Function2::forward_left_null(BasicType::Integer),
Function2::forward_right_null(BasicType::Integer),
Function2::forward_left_null(BasicType::Null),
Function2::forward_right_null(BasicType::Null),
],
),
(
Expand Down Expand Up @@ -1059,6 +1082,12 @@ fn function2_registry() -> HashMap<Func2Type, Vec<Function2>> {
type_out: Type::unencoded(BasicType::Boolean).mutable(),
encoding_invariance: true,
},
Function2::forward_left_null(BasicType::Float),
Function2::forward_right_null(BasicType::Float),
Function2::forward_left_null(BasicType::Integer),
Function2::forward_right_null(BasicType::Integer),
Function2::forward_left_null(BasicType::Null),
Function2::forward_right_null(BasicType::Null),
],
),
(
Expand Down Expand Up @@ -1095,6 +1124,12 @@ fn function2_registry() -> HashMap<Func2Type, Vec<Function2>> {
type_out: Type::unencoded(BasicType::Boolean).mutable(),
encoding_invariance: true,
},
Function2::forward_left_null(BasicType::Float),
Function2::forward_right_null(BasicType::Float),
Function2::forward_left_null(BasicType::Integer),
Function2::forward_right_null(BasicType::Integer),
Function2::forward_left_null(BasicType::Null),
Function2::forward_right_null(BasicType::Null),
],
),
]
Expand Down Expand Up @@ -1138,6 +1173,7 @@ impl QueryPlan {
Filter::NullableU8(filter) => {
planner.null_vec_like(filter.into(), 2, EncodingType::Null)
}
Filter::Null => planner.null_vec(0, EncodingType::Null),
Filter::Indices(filter) => {
planner.null_vec_like(filter.into(), 0, EncodingType::Null)
}
Expand Down Expand Up @@ -2285,6 +2321,7 @@ pub(super) fn prepare<'a>(
std::mem::replace(&mut constant_vecs[index], empty_data(1)),
constant_vec.any(),
),
QueryPlan::Empty { empty, } => operator::empty(empty)?,
QueryPlan::Collect {
input,
collected,
Expand Down
16 changes: 16 additions & 0 deletions tests/query_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1958,3 +1958,19 @@ fn test_negative_constant() {
&[vec![Int(0)], vec![Int(1)]],
);
}

#[test]
fn test_select_where_nullable_gt_constant() {
test_query_ec(
"SELECT id FROM default WHERE nullable_float > 0.1;",
&[vec![Int(2)], vec![Int(9)]],
);
}

#[test]
fn test_sum_where_nullable_gte_constant() {
test_query_ec(
"SELECT SUM(id) FROM default WHERE nullable_float >= 0.1;",
&[vec![Int(11)]],
);
}

0 comments on commit 49fdcbe

Please sign in to comment.