From dacb9a848e31a825fe706f6f7313d95897764932 Mon Sep 17 00:00:00 2001 From: dantengsky Date: Tue, 28 Jun 2022 14:56:03 +0800 Subject: [PATCH 1/3] fix: deletion of null values --- .../src/kernels/data_block_filter.rs | 19 ++++++++-- query/src/interpreters/interpreter_delete.rs | 4 +- .../fuse/operations/mutation/block_filter.rs | 37 +++++++++++++------ .../mutation/mutations_collector.rs | 9 ++--- .../03_dml/03_0025_delete_from.result | 15 ++++++++ .../03_dml/03_0025_delete_from.sql | 31 ++++++++++++++++ .../03_dml/03_0025_delete_from_v2.result | 15 ++++++++ .../03_dml/03_0025_delete_from_v2.sql | 31 ++++++++++++++++ 8 files changed, 138 insertions(+), 23 deletions(-) diff --git a/common/datablocks/src/kernels/data_block_filter.rs b/common/datablocks/src/kernels/data_block_filter.rs index a15404fa5686..7e818f7c9a00 100644 --- a/common/datablocks/src/kernels/data_block_filter.rs +++ b/common/datablocks/src/kernels/data_block_filter.rs @@ -29,16 +29,29 @@ impl DataBlock { let predict_boolean_nonull = Self::cast_to_nonull_boolean(predicate)?; // faster path for constant filter - if predict_boolean_nonull.is_const() { - let flag = predict_boolean_nonull.get_bool(0)?; + if let Some(flag) = Self::try_as_const_bool(&predict_boolean_nonull)? { if flag { return Ok(block); } else { return Ok(DataBlock::empty_with_schema(block.schema().clone())); } } - let boolean_col: &BooleanColumn = Series::check_get(&predict_boolean_nonull)?; + Self::filter_block_with_bool_column(block, boolean_col) + } + + pub fn try_as_const_bool(column_reference: &ColumnRef) -> Result> { + if column_reference.is_const() { + Ok(Some(column_reference.get_bool(0)?)) + } else { + Ok(None) + } + } + + pub fn filter_block_with_bool_column( + block: DataBlock, + boolean_col: &BooleanColumn, + ) -> Result { let rows = boolean_col.len(); let count_zeros = boolean_col.values().null_count(); match count_zeros { diff --git a/query/src/interpreters/interpreter_delete.rs b/query/src/interpreters/interpreter_delete.rs index 87a2bbcce17c..4dee1f369ff8 100644 --- a/query/src/interpreters/interpreter_delete.rs +++ b/query/src/interpreters/interpreter_delete.rs @@ -56,9 +56,7 @@ impl Interpreter for DeleteInterpreter { &self, _input_stream: Option, ) -> Result { - // TODO - // 1. check privilege - // 2. optimize the plan, at least constant folding? + // TODO check privilege let catalog_name = self.plan.catalog_name.as_str(); let db_name = self.plan.database_name.as_str(); let tbl_name = self.plan.table_name.as_str(); diff --git a/query/src/storages/fuse/operations/mutation/block_filter.rs b/query/src/storages/fuse/operations/mutation/block_filter.rs index a8a42cdc70d5..eb54d8da1ba0 100644 --- a/query/src/storages/fuse/operations/mutation/block_filter.rs +++ b/query/src/storages/fuse/operations/mutation/block_filter.rs @@ -12,10 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::ops::Not; use std::sync::Arc; use common_datablocks::DataBlock; +use common_datavalues::BooleanColumn; use common_datavalues::DataSchemaRefExt; +use common_datavalues::Series; use common_exception::Result; use common_planners::Expression; @@ -49,18 +52,13 @@ pub async fn delete_from_block( filter_column_ids } }; + // read the cols that we are going to filtering on let reader = table.create_block_reader(ctx, col_ids)?; let data_block = reader.read_with_block_meta(block_meta).await?; - // inverse the expr - let inverse_expr = Expression::UnaryExpression { - op: "not".to_string(), - expr: Box::new(filter_expr.clone()), - }; - let schema = table.table_info.schema(); - let expr_field = inverse_expr.to_data_field(&schema)?; + let expr_field = filter_expr.to_data_field(&schema)?; let expr_schema = DataSchemaRefExt::create(vec![expr_field]); let expr_exec = ExpressionExecutor::try_create( @@ -68,13 +66,28 @@ pub async fn delete_from_block( "filter expression executor (delete) ", schema.clone(), expr_schema, - vec![inverse_expr], + vec![filter_expr.clone()], false, )?; - - // get the single col data block, which indicates the rows should be kept/removed let filter_result = expr_exec.execute(&data_block)?; + let predicates = DataBlock::cast_to_nonull_boolean(filter_result.column(0))?; + if predicates.is_const() { + let flag = predicates.get_bool(0)?; + return if flag { + Ok(Deletion::Remains(DataBlock::empty_with_schema( + data_block.schema().clone(), + ))) + } else { + //return Ok(DataBlock::empty_with_schema(data_block.schema().clone())); + Ok(Deletion::NothingDeleted) + }; + } + + let boolean_col: &BooleanColumn = Series::check_get(&predicates)?; + let values = boolean_col.values(); + let boolean_col = BooleanColumn::from_arrow_data(values.not()); + // read the whole block let whole_block = if filtering_whole_block { data_block @@ -85,7 +98,9 @@ pub async fn delete_from_block( }; // returns the data remains after deletion - let data_block = DataBlock::filter_block(whole_block, filter_result.column(0))?; + //let data_block = DataBlock::filter_block_with_bool_column(whole_block, filter_result.column(0))?; + let data_block = DataBlock::filter_block_with_bool_column(whole_block, &boolean_col)?; + let res = if data_block.num_rows() == block_meta.row_count as usize { Deletion::NothingDeleted } else { diff --git a/query/src/storages/fuse/operations/mutation/mutations_collector.rs b/query/src/storages/fuse/operations/mutation/mutations_collector.rs index b42b84d0d2f1..85910cb97198 100644 --- a/query/src/storages/fuse/operations/mutation/mutations_collector.rs +++ b/query/src/storages/fuse/operations/mutation/mutations_collector.rs @@ -20,6 +20,7 @@ use common_exception::Result; use opendal::Operator; use crate::sessions::QueryContext; +use crate::storages::fuse::io::write_meta; use crate::storages::fuse::io::BlockWriter; use crate::storages::fuse::io::MetaReaders; use crate::storages::fuse::io::SegmentWriter; @@ -133,16 +134,12 @@ impl<'a> DeletionCollector<'a> { let new_summary = reduce_statistics(&new_segment_summaries)?; new_snapshot.summary = new_summary; - // write the new segment out (and keep it in undo log) + // write the new snapshot out (and keep it in undo log) let snapshot_loc = self.location_generator.snapshot_location_from_uuid( &new_snapshot.snapshot_id, new_snapshot.format_version(), )?; - let bytes = serde_json::to_vec(&new_snapshot)?; - self.data_accessor - .object(&snapshot_loc) - .write(bytes) - .await?; + write_meta(&self.data_accessor, &snapshot_loc, &new_snapshot).await?; Ok((new_snapshot, snapshot_loc)) } diff --git a/tests/suites/0_stateless/03_dml/03_0025_delete_from.result b/tests/suites/0_stateless/03_dml/03_0025_delete_from.result index 23850c27d8b5..bf8010089cb8 100644 --- a/tests/suites/0_stateless/03_dml/03_0025_delete_from.result +++ b/tests/suites/0_stateless/03_dml/03_0025_delete_from.result @@ -7,3 +7,18 @@ other rows should be kept 1 deleted unconditionally 1 +nullable column cases +delete with condition "const false" -- all 3 rows should be kept +1 +normal deletion +expects one row deleted +1 +expects null valued row kept +1 +delete all null valued rows +expects no null valued row kept +1 +expects 1 null valued row kept +1 +deleted unconditionally (with const true) +1 diff --git a/tests/suites/0_stateless/03_dml/03_0025_delete_from.sql b/tests/suites/0_stateless/03_dml/03_0025_delete_from.sql index 9ea625c03b05..8d35f9cfaabc 100644 --- a/tests/suites/0_stateless/03_dml/03_0025_delete_from.sql +++ b/tests/suites/0_stateless/03_dml/03_0025_delete_from.sql @@ -22,4 +22,35 @@ select 'deleted unconditionally'; delete from t; select count(*) = 0 from t; +drop table t all; + +-- setup +select 'nullable column cases'; +create table t (c Int null); +insert into t values (1),(2),(NULL); + +select 'delete with condition "const false" -- all 3 rows should be kept'; +delete from t where 1 = 0; +select count(*) = 3 from t; + +select 'normal deletion'; +delete from t where c = 1; +select 'expects one row deleted'; +select count(*) = 2 from t; +select 'expects null valued row kept'; +select count(*) = 1 from t where c IS NULL; + +select 'delete all null valued rows'; +delete from t where c IS NULL; +select 'expects no null valued row kept'; +select count(*) = 0 from t where c IS NULL; +select 'expects 1 null valued row kept'; +select count(*) = 1 from t where c IS NOT NULL; + +select 'deleted unconditionally (with const true)'; +delete from t where 1 = 1; +select count(*) = 0 from t; + +drop table t all; + DROP DATABASE db1; diff --git a/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.result b/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.result index 23850c27d8b5..bf8010089cb8 100644 --- a/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.result +++ b/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.result @@ -7,3 +7,18 @@ other rows should be kept 1 deleted unconditionally 1 +nullable column cases +delete with condition "const false" -- all 3 rows should be kept +1 +normal deletion +expects one row deleted +1 +expects null valued row kept +1 +delete all null valued rows +expects no null valued row kept +1 +expects 1 null valued row kept +1 +deleted unconditionally (with const true) +1 diff --git a/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.sql b/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.sql index ed8cc34b6c2f..d312e65f4366 100644 --- a/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.sql +++ b/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.sql @@ -24,4 +24,35 @@ select 'deleted unconditionally'; delete from t; select count(*) = 0 from t; +drop table t all; + +-- setup +select 'nullable column cases'; +create table t (c Int null); +insert into t values (1),(2),(NULL); + +select 'delete with condition "const false" -- all 3 rows should be kept'; +delete from t where 1 = 0; +select count(*) = 3 from t; + +select 'normal deletion'; +delete from t where c = 1; +select 'expects one row deleted'; +select count(*) = 2 from t; +select 'expects null valued row kept'; +select count(*) = 1 from t where c IS NULL; + +select 'delete all null valued rows'; +delete from t where c IS NULL; +select 'expects no null valued row kept'; +select count(*) = 0 from t where c IS NULL; +select 'expects 1 null valued row kept'; +select count(*) = 1 from t where c IS NOT NULL; + +select 'deleted unconditionally (with const true)'; +delete from t where 1 = 1; +select count(*) = 0 from t; + +drop table t all; + DROP DATABASE db1; From 31c1e0d959cef61489fc1d16929c57a54577f2a4 Mon Sep 17 00:00:00 2001 From: dantengsky Date: Tue, 28 Jun 2022 22:20:09 +0800 Subject: [PATCH 2/3] add ut and it --- .../src/kernels/data_block_filter.rs | 18 +++++------ .../tests/it/kernels/data_block_filter.rs | 30 +++++++++++++++++++ .../src/storages/fuse/io/read/block_reader.rs | 1 - query/src/storages/fuse/operations/mod.rs | 1 + .../fuse/operations/mutation/block_filter.rs | 24 ++++++++------- .../03_dml/03_0025_delete_from.result | 4 ++- .../03_dml/03_0025_delete_from.sql | 7 ++++- .../03_dml/03_0025_delete_from_v2.result | 4 ++- .../03_dml/03_0025_delete_from_v2.sql | 7 ++++- 9 files changed, 72 insertions(+), 24 deletions(-) diff --git a/common/datablocks/src/kernels/data_block_filter.rs b/common/datablocks/src/kernels/data_block_filter.rs index 7e818f7c9a00..76a9e2045920 100644 --- a/common/datablocks/src/kernels/data_block_filter.rs +++ b/common/datablocks/src/kernels/data_block_filter.rs @@ -29,12 +29,12 @@ impl DataBlock { let predict_boolean_nonull = Self::cast_to_nonull_boolean(predicate)?; // faster path for constant filter - if let Some(flag) = Self::try_as_const_bool(&predict_boolean_nonull)? { - if flag { - return Ok(block); + if let Ok(Some(const_bool)) = Self::try_as_const_bool(&predict_boolean_nonull) { + return if const_bool { + Ok(block) } else { - return Ok(DataBlock::empty_with_schema(block.schema().clone())); - } + Ok(DataBlock::empty_with_schema(block.schema().clone())) + }; } let boolean_col: &BooleanColumn = Series::check_get(&predict_boolean_nonull)?; Self::filter_block_with_bool_column(block, boolean_col) @@ -50,10 +50,10 @@ impl DataBlock { pub fn filter_block_with_bool_column( block: DataBlock, - boolean_col: &BooleanColumn, + filter: &BooleanColumn, ) -> Result { - let rows = boolean_col.len(); - let count_zeros = boolean_col.values().null_count(); + let rows = filter.len(); + let count_zeros = filter.values().null_count(); match count_zeros { 0 => Ok(block), _ => { @@ -62,7 +62,7 @@ impl DataBlock { } let mut after_columns = Vec::with_capacity(block.num_columns()); for data_column in block.columns() { - after_columns.push(data_column.filter(boolean_col)); + after_columns.push(data_column.filter(filter)); } Ok(DataBlock::create(block.schema().clone(), after_columns)) diff --git a/common/datablocks/tests/it/kernels/data_block_filter.rs b/common/datablocks/tests/it/kernels/data_block_filter.rs index 82f88b52e7aa..9e134f270191 100644 --- a/common/datablocks/tests/it/kernels/data_block_filter.rs +++ b/common/datablocks/tests/it/kernels/data_block_filter.rs @@ -139,3 +139,33 @@ fn test_filter_all_const_data_block() -> Result<()> { Ok(()) } + +#[test] +fn test_filter_try_as_const_bool() -> Result<()> { + { + fn const_val(v: bool) { + let const_col = ConstColumn::new(Series::from_data(vec![v]), 6); + let predicate: Arc = Arc::new(const_col); + let block = DataBlock::try_as_const_bool(&predicate); + assert!(matches!(block, Ok(Some(p)) if p == v)); + } + // const values, should return Some(val) + const_val(true); + const_val(false); + } + { + // non-const value, should return None + let predicate = Series::from_data(vec![false]); + let block = DataBlock::try_as_const_bool(&predicate); + assert!(matches!(block, Ok(None))); + } + + { + // const non-bool column , should return Err + let const_col = ConstColumn::new(Series::from_data(vec![vec![1]]), 6); + let predicate: Arc = Arc::new(const_col); + let block = DataBlock::try_as_const_bool(&predicate); + assert!(matches!(block, Err(_))); + } + Ok(()) +} diff --git a/query/src/storages/fuse/io/read/block_reader.rs b/query/src/storages/fuse/io/read/block_reader.rs index 6a661abdc6f8..2f071c7ea529 100644 --- a/query/src/storages/fuse/io/read/block_reader.rs +++ b/query/src/storages/fuse/io/read/block_reader.rs @@ -152,7 +152,6 @@ impl BlockReader { let idx = *col_idx[i]; let field = self.arrow_schema.fields[idx].clone(); let column_descriptor = &self.parquet_schema_descriptor.columns()[idx]; - //let column_meta = &part.columns_meta[&idx]; let column_meta = &meta .col_metas .get(&(i as u32)) diff --git a/query/src/storages/fuse/operations/mod.rs b/query/src/storages/fuse/operations/mod.rs index 77a738ec1516..372a303f1479 100644 --- a/query/src/storages/fuse/operations/mod.rs +++ b/query/src/storages/fuse/operations/mod.rs @@ -27,6 +27,7 @@ mod truncate; pub mod util; pub use fuse_sink::FuseTableSink; +pub use mutation::delete_from_block; pub use operation_log::AppendOperationLogEntry; pub use operation_log::TableOperationLog; pub use util::column_metas; diff --git a/query/src/storages/fuse/operations/mutation/block_filter.rs b/query/src/storages/fuse/operations/mutation/block_filter.rs index eb54d8da1ba0..f3aac1325658 100644 --- a/query/src/storages/fuse/operations/mutation/block_filter.rs +++ b/query/src/storages/fuse/operations/mutation/block_filter.rs @@ -46,7 +46,8 @@ pub async fn delete_from_block( // - if the `filter_expr` is of "constant" function: // for the whole block, whether all of the rows should be kept or dropped // - but, the expr may NOT be deterministic, e.g. - // A nullary non-constant func, which returns true, false or NULL randomly + // A nullary non-constant func, which returns (values convertable to) true, false or NULL randomly + // e.g. delete from t now() all_the_columns_ids(table) } else { filter_column_ids @@ -61,6 +62,7 @@ pub async fn delete_from_block( let expr_field = filter_expr.to_data_field(&schema)?; let expr_schema = DataSchemaRefExt::create(vec![expr_field]); + // get the filter let expr_exec = ExpressionExecutor::try_create( ctx.clone(), "filter expression executor (delete) ", @@ -72,23 +74,25 @@ pub async fn delete_from_block( let filter_result = expr_exec.execute(&data_block)?; let predicates = DataBlock::cast_to_nonull_boolean(filter_result.column(0))?; - if predicates.is_const() { - let flag = predicates.get_bool(0)?; - return if flag { + // shortcut, if predicates is const boolean (or can be cast to boolean) + if let Some(const_bool) = DataBlock::try_as_const_bool(&predicates)? { + return if const_bool { + // all the rows should be removed Ok(Deletion::Remains(DataBlock::empty_with_schema( data_block.schema().clone(), ))) } else { - //return Ok(DataBlock::empty_with_schema(data_block.schema().clone())); + // none of the rows should be removed Ok(Deletion::NothingDeleted) }; } + // reverse the filter let boolean_col: &BooleanColumn = Series::check_get(&predicates)?; let values = boolean_col.values(); - let boolean_col = BooleanColumn::from_arrow_data(values.not()); + let filter = BooleanColumn::from_arrow_data(values.not()); - // read the whole block + // read the whole block if necessary let whole_block = if filtering_whole_block { data_block } else { @@ -97,11 +101,11 @@ pub async fn delete_from_block( whole_block_reader.read_with_block_meta(block_meta).await? }; - // returns the data remains after deletion - //let data_block = DataBlock::filter_block_with_bool_column(whole_block, filter_result.column(0))?; - let data_block = DataBlock::filter_block_with_bool_column(whole_block, &boolean_col)?; + // filter out rows + let data_block = DataBlock::filter_block_with_bool_column(whole_block, &filter)?; let res = if data_block.num_rows() == block_meta.row_count as usize { + // false positive, nothing removed indeed Deletion::NothingDeleted } else { Deletion::Remains(data_block) diff --git a/tests/suites/0_stateless/03_dml/03_0025_delete_from.result b/tests/suites/0_stateless/03_dml/03_0025_delete_from.result index bf8010089cb8..ffc480c20949 100644 --- a/tests/suites/0_stateless/03_dml/03_0025_delete_from.result +++ b/tests/suites/0_stateless/03_dml/03_0025_delete_from.result @@ -8,7 +8,7 @@ other rows should be kept deleted unconditionally 1 nullable column cases -delete with condition "const false" -- all 3 rows should be kept +delete with condition "const false" 1 normal deletion expects one row deleted @@ -22,3 +22,5 @@ expects 1 null valued row kept 1 deleted unconditionally (with const true) 1 +deleted with nullary expr now() +1 diff --git a/tests/suites/0_stateless/03_dml/03_0025_delete_from.sql b/tests/suites/0_stateless/03_dml/03_0025_delete_from.sql index 8d35f9cfaabc..ce6edd1b0c28 100644 --- a/tests/suites/0_stateless/03_dml/03_0025_delete_from.sql +++ b/tests/suites/0_stateless/03_dml/03_0025_delete_from.sql @@ -29,7 +29,7 @@ select 'nullable column cases'; create table t (c Int null); insert into t values (1),(2),(NULL); -select 'delete with condition "const false" -- all 3 rows should be kept'; +select 'delete with condition "const false"'; delete from t where 1 = 0; select count(*) = 3 from t; @@ -51,6 +51,11 @@ select 'deleted unconditionally (with const true)'; delete from t where 1 = 1; select count(*) = 0 from t; +insert into t values (1),(2),(NULL); +select 'deleted with nullary expr now()'; +delete from t where now(); +select count(*) = 0 from t; + drop table t all; DROP DATABASE db1; diff --git a/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.result b/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.result index bf8010089cb8..ffc480c20949 100644 --- a/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.result +++ b/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.result @@ -8,7 +8,7 @@ other rows should be kept deleted unconditionally 1 nullable column cases -delete with condition "const false" -- all 3 rows should be kept +delete with condition "const false" 1 normal deletion expects one row deleted @@ -22,3 +22,5 @@ expects 1 null valued row kept 1 deleted unconditionally (with const true) 1 +deleted with nullary expr now() +1 diff --git a/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.sql b/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.sql index d312e65f4366..d3c69da8e9b6 100644 --- a/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.sql +++ b/tests/suites/0_stateless/03_dml/03_0025_delete_from_v2.sql @@ -31,7 +31,7 @@ select 'nullable column cases'; create table t (c Int null); insert into t values (1),(2),(NULL); -select 'delete with condition "const false" -- all 3 rows should be kept'; +select 'delete with condition "const false"'; delete from t where 1 = 0; select count(*) = 3 from t; @@ -53,6 +53,11 @@ select 'deleted unconditionally (with const true)'; delete from t where 1 = 1; select count(*) = 0 from t; +insert into t values (1),(2),(NULL); +select 'deleted with nullary expr now()'; +delete from t where now(); +select count(*) = 0 from t; + drop table t all; DROP DATABASE db1; From 422836616947bfea31b1d96181dc3dea6faf7157 Mon Sep 17 00:00:00 2001 From: dantengsky Date: Tue, 28 Jun 2022 23:17:07 +0800 Subject: [PATCH 3/3] tidy up --- .../tests/it/kernels/data_block_filter.rs | 12 ++++++------ .../fuse/operations/mutation/block_filter.rs | 16 +++++++++------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/common/datablocks/tests/it/kernels/data_block_filter.rs b/common/datablocks/tests/it/kernels/data_block_filter.rs index 9e134f270191..ae1610f183e0 100644 --- a/common/datablocks/tests/it/kernels/data_block_filter.rs +++ b/common/datablocks/tests/it/kernels/data_block_filter.rs @@ -146,8 +146,8 @@ fn test_filter_try_as_const_bool() -> Result<()> { fn const_val(v: bool) { let const_col = ConstColumn::new(Series::from_data(vec![v]), 6); let predicate: Arc = Arc::new(const_col); - let block = DataBlock::try_as_const_bool(&predicate); - assert!(matches!(block, Ok(Some(p)) if p == v)); + let r = DataBlock::try_as_const_bool(&predicate); + assert!(matches!(r, Ok(Some(p)) if p == v)); } // const values, should return Some(val) const_val(true); @@ -156,16 +156,16 @@ fn test_filter_try_as_const_bool() -> Result<()> { { // non-const value, should return None let predicate = Series::from_data(vec![false]); - let block = DataBlock::try_as_const_bool(&predicate); - assert!(matches!(block, Ok(None))); + let r = DataBlock::try_as_const_bool(&predicate); + assert!(matches!(r, Ok(None))); } { // const non-bool column , should return Err let const_col = ConstColumn::new(Series::from_data(vec![vec![1]]), 6); let predicate: Arc = Arc::new(const_col); - let block = DataBlock::try_as_const_bool(&predicate); - assert!(matches!(block, Err(_))); + let r = DataBlock::try_as_const_bool(&predicate); + assert!(matches!(r, Err(_))); } Ok(()) } diff --git a/query/src/storages/fuse/operations/mutation/block_filter.rs b/query/src/storages/fuse/operations/mutation/block_filter.rs index f3aac1325658..ef070aa5545a 100644 --- a/query/src/storages/fuse/operations/mutation/block_filter.rs +++ b/query/src/storages/fuse/operations/mutation/block_filter.rs @@ -40,14 +40,16 @@ pub async fn delete_from_block( // extract the columns that are going to be filtered on let col_ids = { if filter_column_ids.is_empty() { + // here the situation: filter_expr is not null, but filter_column_ids in not empty, which + // indicates the expr being evaluated is unrelated to the value of rows: + // e.g. + // `delete from t where 1 = 1`, `delete from t where now()`, + // or `delete from t where RANDOM()::INT::BOOLEAN` + // tobe refined: + // if the `filter_expr` is of "constant" nullary : + // for the whole block, whether all of the rows should be kept or dropped, + // we can just return from here, without accessing the block data filtering_whole_block = true; - // To be optimized (in `interpreter_delete`, if we adhere the style of interpreter_select) - // In this case, the expr being evaluated is unrelated to the value of rows: - // - if the `filter_expr` is of "constant" function: - // for the whole block, whether all of the rows should be kept or dropped - // - but, the expr may NOT be deterministic, e.g. - // A nullary non-constant func, which returns (values convertable to) true, false or NULL randomly - // e.g. delete from t now() all_the_columns_ids(table) } else { filter_column_ids