From 4b4c8e6f433cae74b2f9c69a56fbcbb7006034d3 Mon Sep 17 00:00:00 2001 From: Kould <2435992353@qq.com> Date: Fri, 9 Feb 2024 14:25:02 +0800 Subject: [PATCH] feat: support `BETWEEN` on `Where` (#133) * style: pass clippy * feat: support `BETWEEN` on `Where` * code fmt --- src/bin/server.rs | 11 ++-- src/binder/aggregate.rs | 22 ++++++- src/binder/expr.rs | 11 ++++ src/binder/select.rs | 4 +- src/catalog/table.rs | 9 ++- src/db.rs | 2 +- .../codegen/dql/aggregate/hash_agg.rs | 2 +- src/execution/volcano/ddl/add_column.rs | 4 +- src/execution/volcano/dml/analyze.rs | 2 +- .../volcano/dql/aggregate/hash_agg.rs | 11 ++-- src/execution/volcano/dql/join/hash_join.rs | 14 ++--- src/execution/volcano/dql/sort.rs | 6 +- src/expression/evaluator.rs | 34 ++++++++++- src/expression/mod.rs | 47 +++++++++++---- src/expression/simplify.rs | 57 ++++++++++++++---- src/expression/value_compute.rs | 2 +- src/optimizer/core/cm_sketch.rs | 2 +- src/optimizer/core/column_meta.rs | 6 +- src/optimizer/core/histogram.rs | 12 ++-- src/optimizer/core/memo.rs | 2 +- src/optimizer/heuristic/graph.rs | 4 +- src/optimizer/heuristic/optimizer.rs | 7 +-- src/optimizer/rule/implementation/dql/scan.rs | 2 +- .../rule/normalization/combine_operators.rs | 4 +- src/planner/operator/copy_from_file.rs | 2 +- src/planner/operator/create_table.rs | 2 +- src/planner/operator/scan.rs | 2 +- src/planner/operator/values.rs | 2 +- src/storage/kip.rs | 4 +- src/storage/mod.rs | 32 +++++----- src/types/tuple.rs | 8 +-- tests/slt/filter.slt | 60 +++++++++++++++++++ tests/slt/sql_2016/E061_02.slt | 24 +++++--- 33 files changed, 293 insertions(+), 120 deletions(-) diff --git a/src/bin/server.rs b/src/bin/server.rs index f37c463b..cafef1fd 100644 --- a/src/bin/server.rs +++ b/src/bin/server.rs @@ -113,7 +113,7 @@ impl SimpleQueryHandler for SessionBackend { .map_err(|e| PgWireError::ApiError(Box::new(e)))?; guard.replace(transaction); - Ok(vec![Response::Execution(Tag::new("OK").into())]) + Ok(vec![Response::Execution(Tag::new("OK"))]) } "COMMIT;" | "COMMIT" | "COMMIT WORK;" | "COMMIT WORK" => { let mut guard = self.tx.lock().await; @@ -124,7 +124,7 @@ impl SimpleQueryHandler for SessionBackend { .await .map_err(|e| PgWireError::ApiError(Box::new(e)))?; - Ok(vec![Response::Execution(Tag::new("OK").into())]) + Ok(vec![Response::Execution(Tag::new("OK"))]) } else { Err(PgWireError::ApiError(Box::new( DatabaseError::NoTransactionBegin, @@ -141,7 +141,7 @@ impl SimpleQueryHandler for SessionBackend { } drop(guard.take()); - Ok(vec![Response::Execution(Tag::new("OK").into())]) + Ok(vec![Response::Execution(Tag::new("OK"))]) } _ => { let mut guard = self.tx.lock().await; @@ -210,10 +210,7 @@ fn encode_tuples<'a>(tuples: Vec) -> PgWireResult> { results.push(encoder.finish()); } - Ok(QueryResponse::new( - schema, - stream::iter(results.into_iter()), - )) + Ok(QueryResponse::new(schema, stream::iter(results))) } fn into_pg_type(data_type: &LogicalType) -> PgWireResult { diff --git a/src/binder/aggregate.rs b/src/binder/aggregate.rs index fd07d176..80bd8a6e 100644 --- a/src/binder/aggregate.rs +++ b/src/binder/aggregate.rs @@ -110,6 +110,16 @@ impl<'a, T: Transaction> Binder<'a, T> { self.visit_column_agg_expr(arg)?; } } + ScalarExpression::Between { + expr, + left_expr, + right_expr, + .. + } => { + self.visit_column_agg_expr(expr)?; + self.visit_column_agg_expr(left_expr)?; + self.visit_column_agg_expr(right_expr)?; + } ScalarExpression::Constant(_) | ScalarExpression::ColumnRef { .. } => {} } @@ -257,7 +267,17 @@ impl<'a, T: Transaction> Binder<'a, T> { self.validate_having_orderby(right_expr)?; Ok(()) } - + ScalarExpression::Between { + expr, + left_expr, + right_expr, + .. + } => { + self.validate_having_orderby(expr)?; + self.validate_having_orderby(left_expr)?; + self.validate_having_orderby(right_expr)?; + Ok(()) + } ScalarExpression::Constant(_) => Ok(()), } } diff --git a/src/binder/expr.rs b/src/binder/expr.rs index 431d6186..adf49ca3 100644 --- a/src/binder/expr.rs +++ b/src/binder/expr.rs @@ -57,6 +57,17 @@ impl<'a, T: Transaction> Binder<'a, T> { Ok(ScalarExpression::Constant(Arc::new(value))) } + Expr::Between { + expr, + negated, + low, + high, + } => Ok(ScalarExpression::Between { + negated: *negated, + expr: Box::new(self.bind_expr(expr)?), + left_expr: Box::new(self.bind_expr(low)?), + right_expr: Box::new(self.bind_expr(high)?), + }), _ => { todo!() } diff --git a/src/binder/select.rs b/src/binder/select.rs index 09c05001..56ee0738 100644 --- a/src/binder/select.rs +++ b/src/binder/select.rs @@ -408,7 +408,7 @@ impl<'a, T: Transaction> Binder<'a, T> { ScalarExpression::Constant(dv) => match dv.as_ref() { DataValue::Int32(Some(v)) if *v >= 0 => limit = Some(*v as usize), DataValue::Int64(Some(v)) if *v >= 0 => limit = Some(*v as usize), - _ => return Err(DatabaseError::from(DatabaseError::InvalidType)), + _ => return Err(DatabaseError::InvalidType), }, _ => { return Err(DatabaseError::InvalidColumn( @@ -424,7 +424,7 @@ impl<'a, T: Transaction> Binder<'a, T> { ScalarExpression::Constant(dv) => match dv.as_ref() { DataValue::Int32(Some(v)) if *v > 0 => offset = Some(*v as usize), DataValue::Int64(Some(v)) if *v > 0 => offset = Some(*v as usize), - _ => return Err(DatabaseError::from(DatabaseError::InvalidType)), + _ => return Err(DatabaseError::InvalidType), }, _ => { return Err(DatabaseError::InvalidColumn( diff --git a/src/catalog/table.rs b/src/catalog/table.rs index cb2653ff..d2b7fc74 100644 --- a/src/catalog/table.rs +++ b/src/catalog/table.rs @@ -53,7 +53,7 @@ impl TableCatalog { } pub(crate) fn clone_columns(&self) -> Vec { - self.columns.values().map(Arc::clone).collect() + self.columns.values().cloned().collect() } pub(crate) fn columns_with_id(&self) -> Iter<'_, ColumnId, ColumnRef> { @@ -66,8 +66,7 @@ impl TableCatalog { pub(crate) fn primary_key(&self) -> Result<(usize, &ColumnRef), DatabaseError> { self.columns - .iter() - .map(|(_, column)| column) + .values() .enumerate() .find(|(_, column)| column.desc.is_primary) .ok_or(DatabaseError::PrimaryKeyNotFound) @@ -75,8 +74,8 @@ impl TableCatalog { pub(crate) fn types(&self) -> Vec { self.columns - .iter() - .map(|(_, column)| *column.datatype()) + .values() + .map(|column| *column.datatype()) .collect_vec() } diff --git a/src/db.rs b/src/db.rs index ae5560ee..d5691d02 100644 --- a/src/db.rs +++ b/src/db.rs @@ -211,7 +211,7 @@ impl DBTransaction { let (plan, _) = Database::::build_plan::(sql, &self.inner)?; let mut stream = build_write(plan, &mut self.inner); - Ok(try_collect(&mut stream).await?) + try_collect(&mut stream).await } pub async fn commit(self) -> Result<(), DatabaseError> { diff --git a/src/execution/codegen/dql/aggregate/hash_agg.rs b/src/execution/codegen/dql/aggregate/hash_agg.rs index 53bb4b31..034f284e 100644 --- a/src/execution/codegen/dql/aggregate/hash_agg.rs +++ b/src/execution/codegen/dql/aggregate/hash_agg.rs @@ -44,7 +44,7 @@ impl UserData for HashAggStatus { Ok(()) }); methods.add_method_mut("to_tuples", |_, agg_status, ()| { - Ok(agg_status.to_tuples().unwrap()) + Ok(agg_status.as_tuples().unwrap()) }); } } diff --git a/src/execution/volcano/ddl/add_column.rs b/src/execution/volcano/ddl/add_column.rs index 41d0a540..524411ca 100644 --- a/src/execution/volcano/ddl/add_column.rs +++ b/src/execution/volcano/ddl/add_column.rs @@ -35,7 +35,7 @@ impl AddColumn { column, if_not_exists, } = &self.op; - let mut unique_values = column.desc().is_unique.then(|| Vec::new()); + let mut unique_values = column.desc().is_unique.then(Vec::new); let mut tuple_columns = None; let mut tuples = Vec::new(); @@ -78,7 +78,7 @@ impl AddColumn { id: unique_meta.id, column_values: vec![value], }; - transaction.add_index(&table_name, index, vec![tuple_id], true)?; + transaction.add_index(table_name, index, vec![tuple_id], true)?; } } diff --git a/src/execution/volcano/dml/analyze.rs b/src/execution/volcano/dml/analyze.rs index 668d61b3..fabe88ec 100644 --- a/src/execution/volcano/dml/analyze.rs +++ b/src/execution/volcano/dml/analyze.rs @@ -125,7 +125,7 @@ impl fmt::Display for AnalyzeOperator { let columns = self .columns .iter() - .map(|column| format!("{}", column.name())) + .map(|column| column.name().to_string()) .join(", "); write!(f, "Analyze {} -> [{}]", self.table_name, columns)?; diff --git a/src/execution/volcano/dql/aggregate/hash_agg.rs b/src/execution/volcano/dql/aggregate/hash_agg.rs index d25fdb47..61bcedbd 100644 --- a/src/execution/volcano/dql/aggregate/hash_agg.rs +++ b/src/execution/volcano/dql/aggregate/hash_agg.rs @@ -109,11 +109,10 @@ impl HashAggStatus { Ok(()) } - pub(crate) fn to_tuples(&mut self) -> Result, DatabaseError> { - let group_columns = Arc::new(mem::replace(&mut self.group_columns, vec![])); + pub(crate) fn as_tuples(&mut self) -> Result, DatabaseError> { + let group_columns = Arc::new(mem::take(&mut self.group_columns)); - Ok(self - .group_hash_accs + self.group_hash_accs .drain() .map(|(group_keys, accs)| { // Tips: Accumulator First @@ -129,7 +128,7 @@ impl HashAggStatus { values, }) }) - .try_collect()?) + .try_collect() } } @@ -149,7 +148,7 @@ impl HashAggExecutor { agg_status.update(tuple?)?; } - for tuple in agg_status.to_tuples()? { + for tuple in agg_status.as_tuples()? { yield tuple; } } diff --git a/src/execution/volcano/dql/join/hash_join.rs b/src/execution/volcano/dql/join/hash_join.rs index fe32a6e1..ab3c445e 100644 --- a/src/execution/volcano/dql/join/hash_join.rs +++ b/src/execution/volcano/dql/join/hash_join.rs @@ -111,10 +111,7 @@ impl HashJoinStatus { let _ = mem::replace(left_init_flag, true); } - build_map - .entry(hash) - .or_insert_with(|| Vec::new()) - .push(tuple); + build_map.entry(hash).or_insert_with(Vec::new).push(tuple); Ok(()) } @@ -134,7 +131,7 @@ impl HashJoinStatus { } = self; let right_cols_len = tuple.columns.len(); - let hash = Self::hash_row(&on_right_keys, &hash_random_state, &tuple)?; + let hash = Self::hash_row(on_right_keys, hash_random_state, &tuple)?; if !*right_init_flag { Self::columns_filling(&tuple, join_columns, *right_force_nullable); @@ -240,14 +237,14 @@ impl HashJoinStatus { build_map .drain() .filter(|(hash, _)| !used_set.contains(hash)) - .map(|(_, mut tuples)| { + .flat_map(|(_, mut tuples)| { for Tuple { values, columns, id, } in tuples.iter_mut() { - let _ = mem::replace(id, None); + let _ = id.take(); let (right_values, full_columns) = buf.get_or_insert_with(|| { let (right_values, mut right_columns): ( Vec, @@ -269,10 +266,9 @@ impl HashJoinStatus { } tuples }) - .flatten() .collect_vec() }) - .unwrap_or_else(|| vec![]) + .unwrap_or_else(Vec::new) } fn columns_filling(tuple: &Tuple, join_columns: &mut Vec, force_nullable: bool) { diff --git a/src/execution/volcano/dql/sort.rs b/src/execution/volcano/dql/sort.rs index eb122264..0957fe0a 100644 --- a/src/execution/volcano/dql/sort.rs +++ b/src/execution/volcano/dql/sort.rs @@ -26,11 +26,7 @@ pub(crate) fn radix_sort(mut tuples: Vec<(T, Vec)>) -> Vec { temp_buckets[index as usize].push((t, bytes)); } - tuples = temp_buckets - .iter_mut() - .map(|group| mem::replace(group, vec![])) - .flatten() - .collect_vec(); + tuples = temp_buckets.iter_mut().flat_map(mem::take).collect_vec(); } return tuples.into_iter().map(|(tuple, _)| tuple).collect_vec(); } diff --git a/src/expression/evaluator.rs b/src/expression/evaluator.rs index 90b581e9..53cc8bef 100644 --- a/src/expression/evaluator.rs +++ b/src/expression/evaluator.rs @@ -6,6 +6,7 @@ use crate::types::tuple::Tuple; use crate::types::value::{DataValue, ValueRef}; use itertools::Itertools; use lazy_static::lazy_static; +use std::cmp::Ordering; use std::sync::Arc; lazy_static! { @@ -68,9 +69,17 @@ impl ScalarExpression { negated, } => { let value = expr.eval(tuple)?; + if value.is_null() { + return Ok(Arc::new(DataValue::Boolean(None))); + } let mut is_in = false; for arg in args { - if arg.eval(tuple)? == value { + let arg_value = arg.eval(tuple)?; + + if arg_value.is_null() { + return Ok(Arc::new(DataValue::Boolean(None))); + } + if arg_value == value { is_in = true; break; } @@ -92,6 +101,29 @@ impl ScalarExpression { Ok(value) } + ScalarExpression::Between { + expr, + left_expr, + right_expr, + negated, + } => { + let value = expr.eval(tuple)?; + let left = left_expr.eval(tuple)?; + let right = right_expr.eval(tuple)?; + + let mut is_between = match ( + value.partial_cmp(&left).map(Ordering::is_ge), + value.partial_cmp(&right).map(Ordering::is_le), + ) { + (Some(true), Some(true)) => true, + (None, _) | (_, None) => return Ok(Arc::new(DataValue::Boolean(None))), + _ => false, + }; + if *negated { + is_between = !is_between; + } + Ok(Arc::new(DataValue::Boolean(Some(is_between)))) + } } } diff --git a/src/expression/mod.rs b/src/expression/mod.rs index ad15fdbb..d40b91d2 100644 --- a/src/expression/mod.rs +++ b/src/expression/mod.rs @@ -58,6 +58,12 @@ pub enum ScalarExpression { expr: Box, args: Vec, }, + Between { + negated: bool, + expr: Box, + left_expr: Box, + right_expr: Box, + }, } impl ScalarExpression { @@ -99,9 +105,15 @@ impl ScalarExpression { .. } => left_expr.nullable() && right_expr.nullable(), ScalarExpression::In { expr, args, .. } => { - args.iter().all(ScalarExpression::nullable) && expr.nullable() + expr.nullable() && args.iter().all(ScalarExpression::nullable) } ScalarExpression::AggCall { args, .. } => args.iter().all(ScalarExpression::nullable), + ScalarExpression::Between { + expr, + left_expr, + right_expr, + .. + } => expr.nullable() && left_expr.nullable() && right_expr.nullable(), } } @@ -121,7 +133,9 @@ impl ScalarExpression { Self::AggCall { ty: return_type, .. } => *return_type, - Self::IsNull { .. } | Self::In { .. } => LogicalType::Boolean, + Self::IsNull { .. } | Self::In { .. } | ScalarExpression::Between { .. } => { + LogicalType::Boolean + } Self::Alias { expr, .. } => expr.return_type(), } } @@ -194,6 +208,12 @@ impl ScalarExpression { ScalarExpression::In { expr, args, .. } => { expr.has_agg_call() || args.iter().any(|arg| arg.has_agg_call()) } + ScalarExpression::Between { + expr, + left_expr, + right_expr, + .. + } => expr.has_agg_call() || left_expr.has_agg_call() || right_expr.has_agg_call(), } } @@ -248,16 +268,23 @@ impl ScalarExpression { negated, expr, } => { - let args_string = args - .iter() - .map(|arg| arg.output_column().name().to_string()) - .join(", "); + let args_string = args.iter().map(|arg| arg.output_name()).join(", "); let op_string = if *negated { "not in" } else { "in" }; + format!("{} {} ({})", expr.output_name(), op_string, args_string) + } + ScalarExpression::Between { + expr, + left_expr, + right_expr, + negated, + } => { + let op_string = if *negated { "not between" } else { "between" }; format!( - "{} {} ({})", - expr.output_column().name(), + "{} {} [{}, {}]", + expr.output_name(), op_string, - args_string + left_expr.output_name(), + right_expr.output_name() ) } } @@ -320,7 +347,7 @@ pub enum BinaryOperator { impl fmt::Display for ScalarExpression { fn fmt(&self, f: &mut Formatter) -> fmt::Result { - write!(f, "{}", self.output_column().name()) + write!(f, "{}", self.output_name()) } } diff --git a/src/expression/simplify.rs b/src/expression/simplify.rs index 0142a7cb..a0cc5de3 100644 --- a/src/expression/simplify.rs +++ b/src/expression/simplify.rs @@ -198,7 +198,7 @@ impl ConstantBinary { // Tips: It only makes sense if the condition is and aggregation fn and_scope_aggregation( - binaries: &Vec, + binaries: &[ConstantBinary], ) -> Result, DatabaseError> { if binaries.is_empty() { return Ok(vec![]); @@ -254,7 +254,7 @@ impl ConstantBinary { .next() .map(ConstantBinary::Eq); - return if let Some(eq) = eq_option { + if let Some(eq) = eq_option { Ok(vec![eq]) } else if !matches!( (&scope_min, &scope_max), @@ -268,7 +268,7 @@ impl ConstantBinary { Ok(vec![scope_binary]) } else { Ok(vec![]) - }; + } } // Tips: It only makes sense if the condition is or aggregation @@ -346,8 +346,7 @@ impl ConstantBinary { Self::bound_compared(min_a, min_b, true).unwrap() }); - for i in 0..scopes.len() { - let (min, max) = scopes[i]; + for (min, max) in scopes { if merge_scopes.is_empty() { merge_scopes.push((min.clone(), max.clone())); continue; @@ -379,7 +378,7 @@ impl ConstantBinary { .collect_vec() } - fn join_write(f: &mut Formatter, binaries: &Vec, op: &str) -> fmt::Result { + fn join_write(f: &mut Formatter, binaries: &[ConstantBinary], op: &str) -> fmt::Result { let binaries = binaries.iter().map(|binary| format!("{}", binary)).join(op); write!(f, " {} ", binaries)?; @@ -634,7 +633,7 @@ impl ScalarExpression { (BinaryOperator::Eq, BinaryOperator::Or) }; let mut new_expr = ScalarExpression::Binary { - op: op_1.clone(), + op: op_1, left_expr: expr.clone(), right_expr: Box::new(args.remove(0)), ty: LogicalType::Boolean, @@ -642,9 +641,9 @@ impl ScalarExpression { for arg in args.drain(..) { new_expr = ScalarExpression::Binary { - op: op_2.clone(), + op: op_2, left_expr: Box::new(ScalarExpression::Binary { - op: op_1.clone(), + op: op_1, left_expr: expr.clone(), right_expr: Box::new(arg), ty: LogicalType::Boolean, @@ -656,6 +655,40 @@ impl ScalarExpression { let _ = mem::replace(self, new_expr); } + ScalarExpression::Between { + expr, + left_expr, + right_expr, + negated, + } => { + let (op, left_op, right_op) = if *negated { + (BinaryOperator::Or, BinaryOperator::Lt, BinaryOperator::Gt) + } else { + ( + BinaryOperator::And, + BinaryOperator::GtEq, + BinaryOperator::LtEq, + ) + }; + let new_expr = ScalarExpression::Binary { + op, + left_expr: Box::new(ScalarExpression::Binary { + op: left_op, + left_expr: expr.clone(), + right_expr: left_expr.clone(), + ty: LogicalType::Boolean, + }), + right_expr: Box::new(ScalarExpression::Binary { + op: right_op, + left_expr: expr.clone(), + right_expr: right_expr.clone(), + ty: LogicalType::Boolean, + }), + ty: LogicalType::Boolean, + }; + + let _ = mem::replace(self, new_expr); + } _ => (), } @@ -883,7 +916,8 @@ impl ScalarExpression { ScalarExpression::Alias { expr, .. } | ScalarExpression::TypeCast { expr, .. } | ScalarExpression::Unary { expr, .. } - | ScalarExpression::In { expr, .. } => expr.convert_binary(col_id), + | ScalarExpression::In { expr, .. } + | ScalarExpression::Between { expr, .. } => expr.convert_binary(col_id), ScalarExpression::IsNull { expr, negated, .. } => match expr.as_ref() { ScalarExpression::ColumnRef(column) => { Ok(column.id().is_some_and(|id| col_id == &id).then(|| { @@ -901,7 +935,8 @@ impl ScalarExpression { | ScalarExpression::Unary { .. } | ScalarExpression::Binary { .. } | ScalarExpression::AggCall { .. } - | ScalarExpression::In { .. } => expr.convert_binary(col_id), + | ScalarExpression::In { .. } + | ScalarExpression::Between { .. } => expr.convert_binary(col_id), }, ScalarExpression::Constant(_) | ScalarExpression::ColumnRef(_) diff --git a/src/expression/value_compute.rs b/src/expression/value_compute.rs index 8bf2ff8f..77adad4c 100644 --- a/src/expression/value_compute.rs +++ b/src/expression/value_compute.rs @@ -531,7 +531,7 @@ pub fn binary_op( _ => todo!("unsupported operator"), } } - LogicalType::SqlNull => return Err(DatabaseError::NotNull), + LogicalType::SqlNull => return Ok(DataValue::Null), LogicalType::Invalid => return Err(DatabaseError::InvalidType), }; diff --git a/src/optimizer/core/cm_sketch.rs b/src/optimizer/core/cm_sketch.rs index 15d9e78b..b8df970d 100644 --- a/src/optimizer/core/cm_sketch.rs +++ b/src/optimizer/core/cm_sketch.rs @@ -98,7 +98,7 @@ impl CountMinSketch { self.counters[k_i][offset] }) .min() - .unwrap() as usize + .unwrap() } #[allow(dead_code)] diff --git a/src/optimizer/core/column_meta.rs b/src/optimizer/core/column_meta.rs index bffb7978..20ed2dea 100644 --- a/src/optimizer/core/column_meta.rs +++ b/src/optimizer/core/column_meta.rs @@ -28,7 +28,7 @@ impl<'a, T: Transaction> ColumnMetaLoader<'a, T> { pub fn load(&self, table_name: TableName) -> Result<&Vec, DatabaseError> { let option = self.cache.get(&table_name); - return if let Some(column_metas) = option { + if let Some(column_metas) = option { Ok(column_metas) } else { let paths = self.tx.column_meta_paths(&table_name)?; @@ -39,7 +39,7 @@ impl<'a, T: Transaction> ColumnMetaLoader<'a, T> { } Ok(self.cache.get_or_insert(table_name, |_| Ok(column_metas))?) - }; + } } } @@ -87,7 +87,7 @@ impl ColumnMeta { .write(true) .read(true) .open(path)?; - let _ = file.write_all(&bincode::serialize(self)?)?; + file.write_all(&bincode::serialize(self)?)?; file.flush()?; Ok(()) diff --git a/src/optimizer/core/histogram.rs b/src/optimizer/core/histogram.rs index a9983b73..007556eb 100644 --- a/src/optimizer/core/histogram.rs +++ b/src/optimizer/core/histogram.rs @@ -53,9 +53,7 @@ impl HistogramBuilder { column_id: column.id().ok_or(DatabaseError::OwnerLessColumn)?, data_type: *column.datatype(), null_count: 0, - values: capacity - .map(Vec::with_capacity) - .unwrap_or_else(|| Vec::new()), + values: capacity.map(Vec::with_capacity).unwrap_or_default(), value_index: 0, }) } @@ -231,7 +229,7 @@ impl Histogram { while bucket_i < self.buckets.len() && binary_i < binaries.len() { self._collect_count( - &binaries, + binaries, &mut binary_i, &mut bucket_i, &mut bucket_idxs, @@ -259,9 +257,9 @@ impl Histogram { let float_value = |value: &DataValue, prefix_len: usize| { match value.logical_type() { LogicalType::Varchar(_) => match value { - DataValue::Utf8(value) => value.as_ref().and_then(|string| { + DataValue::Utf8(value) => value.as_ref().map(|string| { if prefix_len > string.len() { - return Some(0.0); + return 0.0; } let mut val = 0u64; @@ -278,7 +276,7 @@ impl Histogram { } } - Some(val as f64) + val as f64 }), _ => unreachable!(), }, diff --git a/src/optimizer/core/memo.rs b/src/optimizer/core/memo.rs index 65d109ba..1b6c5796 100644 --- a/src/optimizer/core/memo.rs +++ b/src/optimizer/core/memo.rs @@ -146,7 +146,7 @@ mod tests { ]; let memo = Memo::new(&graph, &transaction.meta_loader(), &rules)?; - let best_plan = graph.to_plan(Some(&memo)); + let best_plan = graph.into_plan(Some(&memo)); let exprs = &memo.groups.get(&NodeIndex::new(3)).unwrap(); assert_eq!(exprs.exprs.len(), 2); diff --git a/src/optimizer/heuristic/graph.rs b/src/optimizer/heuristic/graph.rs index 50cc8276..51aafe11 100644 --- a/src/optimizer/heuristic/graph.rs +++ b/src/optimizer/heuristic/graph.rs @@ -182,7 +182,7 @@ impl HepGraph { .map(|edge| edge.target()) } - pub fn to_plan(mut self, memo: Option<&Memo>) -> Option { + pub fn into_plan(mut self, memo: Option<&Memo>) -> Option { self.build_childrens(self.root_index, memo) } @@ -345,7 +345,7 @@ mod tests { let plan = select_sql_run("select * from t1 left join t2 on c1 = c3").await?; let graph = HepGraph::new(plan.clone()); - let plan_for_graph = graph.to_plan(None).unwrap(); + let plan_for_graph = graph.into_plan(None).unwrap(); assert_eq!(plan, plan_for_graph); diff --git a/src/optimizer/heuristic/optimizer.rs b/src/optimizer/heuristic/optimizer.rs index 415d8e3b..d1a0e514 100644 --- a/src/optimizer/heuristic/optimizer.rs +++ b/src/optimizer/heuristic/optimizer.rs @@ -67,10 +67,9 @@ impl HepOptimizer { }) .transpose()?; - Ok(self - .graph - .to_plan(memo.as_ref()) - .ok_or(DatabaseError::EmptyPlan)?) + self.graph + .into_plan(memo.as_ref()) + .ok_or(DatabaseError::EmptyPlan) } fn apply_batch( diff --git a/src/optimizer/rule/implementation/dql/scan.rs b/src/optimizer/rule/implementation/dql/scan.rs index 58147905..7409cb0b 100644 --- a/src/optimizer/rule/implementation/dql/scan.rs +++ b/src/optimizer/rule/implementation/dql/scan.rs @@ -103,7 +103,7 @@ impl ImplementationRule for IndexScanImplementation { } fn find_column_meta<'a>( - column_metas: &'a Vec, + column_metas: &'a [ColumnMeta], column_id: &ColumnId, ) -> Option<&'a ColumnMeta> { assert!(column_metas.is_sorted_by_key(ColumnMeta::column_id)); diff --git a/src/optimizer/rule/normalization/combine_operators.rs b/src/optimizer/rule/normalization/combine_operators.rs index bcc66105..785b3106 100644 --- a/src/optimizer/rule/normalization/combine_operators.rs +++ b/src/optimizer/rule/normalization/combine_operators.rs @@ -121,7 +121,7 @@ impl NormalizationRule for CollapseGroupByAgg { if let Some(Operator::Aggregate(child_op)) = graph .eldest_child_at(node_id) - .and_then(|child_id| Some(graph.operator_mut(child_id))) + .map(|child_id| graph.operator_mut(child_id)) { if op.groupby_exprs.len() != child_op.groupby_exprs.len() { return Ok(()); @@ -134,7 +134,7 @@ impl NormalizationRule for CollapseGroupByAgg { for expr in child_op.groupby_exprs.iter() { expr_set.remove(expr); } - if expr_set.len() == 0 { + if expr_set.is_empty() { graph.remove_node(node_id, false); } } diff --git a/src/planner/operator/copy_from_file.rs b/src/planner/operator/copy_from_file.rs index c2f00725..02154991 100644 --- a/src/planner/operator/copy_from_file.rs +++ b/src/planner/operator/copy_from_file.rs @@ -16,7 +16,7 @@ impl fmt::Display for CopyFromFileOperator { let columns = self .columns .iter() - .map(|column| format!("{}", column.name())) + .map(|column| column.name().to_string()) .join(", "); write!( f, diff --git a/src/planner/operator/create_table.rs b/src/planner/operator/create_table.rs index 0adca431..b55b0192 100644 --- a/src/planner/operator/create_table.rs +++ b/src/planner/operator/create_table.rs @@ -17,7 +17,7 @@ impl fmt::Display for CreateTableOperator { let columns = self .columns .iter() - .map(|column| format!("{}", column.name())) + .map(|column| column.name().to_string()) .join(", "); write!( f, diff --git a/src/planner/operator/scan.rs b/src/planner/operator/scan.rs index 16095db0..350445c0 100644 --- a/src/planner/operator/scan.rs +++ b/src/planner/operator/scan.rs @@ -64,7 +64,7 @@ impl fmt::Display for ScanOperator { let projection_columns = self .columns .iter() - .map(|(_, column)| format!("{}", column.name())) + .map(|(_, column)| column.name().to_string()) .join(", "); let (offset, limit) = self.limit; diff --git a/src/planner/operator/values.rs b/src/planner/operator/values.rs index e1472287..95510652 100644 --- a/src/planner/operator/values.rs +++ b/src/planner/operator/values.rs @@ -15,7 +15,7 @@ impl fmt::Display for ValuesOperator { let columns = self .columns .iter() - .map(|column| format!("{}", column.name())) + .map(|column| column.name().to_string()) .join(", "); write!(f, "Values [{}], RowsLen: {}", columns, self.rows.len())?; diff --git a/src/storage/kip.rs b/src/storage/kip.rs index 89f786f5..1e61fae6 100644 --- a/src/storage/kip.rs +++ b/src/storage/kip.rs @@ -230,7 +230,7 @@ impl Transaction for KipTransaction { } let column = table.get_column_by_id(&col_id).unwrap(); - let (key, value) = TableCodec::encode_column(&table_name, column)?; + let (key, value) = TableCodec::encode_column(table_name, column)?; self.tx.set(key, value); self.table_cache.remove(table_name); @@ -256,7 +256,7 @@ impl Transaction for KipTransaction { let (index_min, index_max) = TableCodec::index_bound(table_name, &index_meta.id); Self::_drop_data(&mut self.tx, &index_min, &index_max)?; } - let (key, _) = TableCodec::encode_column(&table_name, column)?; + let (key, _) = TableCodec::encode_column(table_name, column)?; match self.tx.remove(&key) { Ok(_) => (), diff --git a/src/storage/mod.rs b/src/storage/mod.rs index e8712258..e32b70c3 100644 --- a/src/storage/mod.rs +++ b/src/storage/mod.rs @@ -147,7 +147,7 @@ impl IndexIter<'_> { } fn get_tuple_by_id(&mut self, tuple_id: &TupleId) -> Result, DatabaseError> { - let key = TableCodec::encode_tuple_key(&self.table.name, &tuple_id)?; + let key = TableCodec::encode_tuple_key(&self.table.name, tuple_id)?; Ok(self.tx.get(&key)?.map(|bytes| { TableCodec::decode_tuple( @@ -174,27 +174,23 @@ impl Iter for IndexIter<'_> { return Ok(None); } // 2. try get tuple on index_values and until it empty - loop { - if let Some(value) = self.index_values.pop_front() { - if Self::offset_move(&mut self.offset) { - continue; - } - match value { - IndexValue::PrimaryKey(tuple) => { - if let Some(num) = self.limit.as_mut() { - num.sub_assign(1); - } + while let Some(value) = self.index_values.pop_front() { + if Self::offset_move(&mut self.offset) { + continue; + } + match value { + IndexValue::PrimaryKey(tuple) => { + if let Some(num) = self.limit.as_mut() { + num.sub_assign(1); + } + return Ok(Some(tuple)); + } + IndexValue::Normal(tuple_id) => { + if let Some(tuple) = self.get_tuple_by_id(&tuple_id)? { return Ok(Some(tuple)); } - IndexValue::Normal(tuple_id) => { - if let Some(tuple) = self.get_tuple_by_id(&tuple_id)? { - return Ok(Some(tuple)); - } - } } - } else { - break; } } assert!(self.index_values.is_empty()); diff --git a/src/types/tuple.rs b/src/types/tuple.rs index d15a701c..b52fa91d 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -41,7 +41,7 @@ impl Tuple { let mut projection_i = 0; let mut pos = bits_len; - for (i, logic_type) in table_types.into_iter().enumerate() { + for (i, logic_type) in table_types.iter().enumerate() { if projection_i >= tuple_columns.len() { break; } @@ -50,7 +50,7 @@ impl Tuple { tuple_values.push(Arc::new(DataValue::none(logic_type))); Self::values_push( tuple_columns, - &mut tuple_values, + &tuple_values, &mut id_option, &mut projection_i, ); @@ -64,7 +64,7 @@ impl Tuple { ))); Self::values_push( tuple_columns, - &mut tuple_values, + &tuple_values, &mut id_option, &mut projection_i, ); @@ -81,7 +81,7 @@ impl Tuple { ))); Self::values_push( tuple_columns, - &mut tuple_values, + &tuple_values, &mut id_option, &mut projection_i, ); diff --git a/tests/slt/filter.slt b/tests/slt/filter.slt index 45cb7fd9..837a0812 100644 --- a/tests/slt/filter.slt +++ b/tests/slt/filter.slt @@ -112,6 +112,66 @@ select * from t1 where id not in (1, 2) 0 KipSQL 3 Cool! +query II +select * from t1 where id in (1, null) +---- + +query II +select * from t1 where null in (1, 2) +---- + +query II +select * from t1 where null in (1, null) +---- + +query II +select * from t1 where id not in (1, null) +---- + +query II +select * from t1 where null not in (1, 2) +---- + +query II +select * from t1 where null not in (1, null) +---- + +query II +select * from t1 where id between 1 and 2 +---- +1 KipDB +2 KipBlog + +query II +select * from t1 where id not between 1 and 2 +---- +0 KipSQL +3 Cool! + +query II +select * from t1 where id between 1 and null +---- + +query II +select * from t1 where null between 1 and 2 +---- + +query II +select * from t1 where null between 1 and null +---- + +query II +select * from t1 where id not between 1 and null +---- + +query II +select * from t1 where null not between 1 and 2 +---- + +query II +select * from t1 where null not between 1 and null +---- + statement ok drop table t diff --git a/tests/slt/sql_2016/E061_02.slt b/tests/slt/sql_2016/E061_02.slt index 74f9a6bd..5f3acaae 100644 --- a/tests/slt/sql_2016/E061_02.slt +++ b/tests/slt/sql_2016/E061_02.slt @@ -1,33 +1,41 @@ # E061-02: BETWEEN predicate -# TODO: Support `BETWEEN` on `Where` +statement ok +CREATE TABLE TABLE_E061_02_01_01 ( ID INT PRIMARY KEY, A INT ); -# statement ok -# CREATE TABLE TABLE_E061_02_01_01 ( ID INT PRIMARY KEY, A INT ); - -# SELECT A FROM TABLE_E061_02_01_01 WHERE A BETWEEN 1 AND 1 +query I +SELECT A FROM TABLE_E061_02_01_01 WHERE A BETWEEN 1 AND 1 # statement ok # CREATE TABLE TABLE_E061_02_01_02 ( ID INT PRIMARY KEY, A INT ); +# sqlparser-rs unsupported +# query I # SELECT A FROM TABLE_E061_02_01_02 WHERE A BETWEEN ASYMMETRIC 1 AND 1 # statement ok # CREATE TABLE TABLE_E061_02_01_03 ( ID INT PRIMARY KEY, A INT ); +# sqlparser-rs unsupported +# query I # SELECT A FROM TABLE_E061_02_01_03 WHERE A BETWEEN SYMMETRIC 1 AND 1 -# statement ok -# CREATE TABLE TABLE_E061_02_01_04 ( ID INT PRIMARY KEY, A INT ); +statement ok +CREATE TABLE TABLE_E061_02_01_04 ( ID INT PRIMARY KEY, A INT ); -# SELECT A FROM TABLE_E061_02_01_04 WHERE A NOT BETWEEN 1 AND 1 +query I +SELECT A FROM TABLE_E061_02_01_04 WHERE A NOT BETWEEN 1 AND 1 # statement ok # CREATE TABLE TABLE_E061_02_01_05 ( ID INT PRIMARY KEY, A INT ); +# sqlparser-rs unsupported +# query I # SELECT A FROM TABLE_E061_02_01_05 WHERE A NOT BETWEEN ASYMMETRIC 1 AND 1 # statement ok # CREATE TABLE TABLE_E061_02_01_06 ( ID INT PRIMARY KEY, A INT ); +# sqlparser-rs unsupported +# query I # SELECT A FROM TABLE_E061_02_01_06 WHERE A NOT BETWEEN SYMMETRIC 1 AND 1