Skip to content

Commit d87e4b1

Browse files
forsaken628forsaken628
authored
refactor(query): AccessType downcasts now return Result so mismatches surface useful diagnostics (#18923)
1 parent 0efebe5 commit d87e4b1

File tree

42 files changed

+677
-463
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+677
-463
lines changed

‎src/query/expression/src/block.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,13 +182,13 @@ impl BlockEntry {
182182
}
183183
}
184184

185-
pub fn downcast<T: AccessType>(&self) -> Option<ColumnView<T>> {
185+
pub fn downcast<T: AccessType>(&self) -> Result<ColumnView<T>> {
186186
match self {
187-
BlockEntry::Const(scalar, _, num_rows) => T::try_downcast_scalar(&scalar.as_ref())
188-
.map(|s| ColumnView::Const(T::to_owned_scalar(s), *num_rows)),
189-
BlockEntry::Column(column) => {
190-
T::try_downcast_column(column).map(|c| ColumnView::Column(c))
191-
}
187+
BlockEntry::Const(scalar, _, num_rows) => Ok(ColumnView::Const(
188+
T::to_owned_scalar(T::try_downcast_scalar(&scalar.as_ref())?),
189+
*num_rows,
190+
)),
191+
BlockEntry::Column(column) => Ok(ColumnView::Column(T::try_downcast_column(column)?)),
192192
}
193193
}
194194
}

‎src/query/expression/src/register.rs‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -912,21 +912,18 @@ fn erase_function_generic_0_arg<O: ArgType>(
912912
fn erase_function_generic_1_arg<I1: ArgType, O: ArgType>(
913913
func: impl for<'a> Fn(Value<I1>, &mut EvalContext) -> Value<O>,
914914
) -> impl Fn(&[Value<AnyType>], &mut EvalContext) -> Value<AnyType> {
915-
move |args, ctx| {
916-
let arg1 = match args[0].try_downcast() {
917-
Some(arg1) => arg1,
918-
None => match &args[0] {
919-
Value::Scalar(scalar) => {
920-
unreachable!("can't downcast from {scalar:?} to {}", I1::data_type())
921-
}
922-
Value::Column(column) => unreachable!(
923-
"can't downcast from {} to {}",
924-
column.data_type(),
925-
I1::data_type()
926-
),
927-
},
928-
};
929-
Value::upcast(func(arg1, ctx))
915+
move |args, ctx| match args[0].try_downcast() {
916+
Ok(arg1) => Value::upcast(func(arg1, ctx)),
917+
Err(_) => match &args[0] {
918+
Value::Scalar(scalar) => {
919+
unreachable!("can't downcast from {scalar:?} to {}", I1::data_type())
920+
}
921+
Value::Column(column) => unreachable!(
922+
"can't downcast from {} to {}",
923+
column.data_type(),
924+
I1::data_type()
925+
),
926+
},
930927
}
931928
}
932929

‎src/query/expression/src/types.rs‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ impl DataType {
277277
DataType::Number(NumberDataType::Int64) => {
278278
Ok(Scalar::Number(NumberScalar::Int64(i64::MAX)))
279279
}
280-
_ => Result::Err(format!(
280+
_ => Err(format!(
281281
"only support numeric types and time types, but got {:?}",
282282
self
283283
)),
@@ -305,7 +305,7 @@ impl DataType {
305305
DataType::Number(NumberDataType::Int64) => {
306306
Ok(Scalar::Number(NumberScalar::Int64(i64::MIN)))
307307
}
308-
_ => Result::Err(format!(
308+
_ => Err(format!(
309309
"only support numeric types and time types, but got {:?}",
310310
self
311311
)),
@@ -579,10 +579,10 @@ pub trait AccessType: Debug + Clone + PartialEq + Sized + 'static {
579579
fn to_owned_scalar(scalar: Self::ScalarRef<'_>) -> Self::Scalar;
580580
fn to_scalar_ref(scalar: &Self::Scalar) -> Self::ScalarRef<'_>;
581581

582-
fn try_downcast_scalar<'a>(scalar: &ScalarRef<'a>) -> Option<Self::ScalarRef<'a>>;
583-
fn try_downcast_domain(domain: &Domain) -> Option<Self::Domain>;
582+
fn try_downcast_scalar<'a>(scalar: &ScalarRef<'a>) -> Result<Self::ScalarRef<'a>, ErrorCode>;
583+
fn try_downcast_domain(domain: &Domain) -> Result<Self::Domain, ErrorCode>;
584584

585-
fn try_downcast_column(col: &Column) -> Option<Self::Column>;
585+
fn try_downcast_column(col: &Column) -> Result<Self::Column, ErrorCode>;
586586

587587
fn column_len(col: &Self::Column) -> usize;
588588
fn index_column(col: &Self::Column, index: usize) -> Option<Self::ScalarRef<'_>>;
@@ -650,6 +650,30 @@ pub trait AccessType: Debug + Clone + PartialEq + Sized + 'static {
650650
}
651651
}
652652

653+
fn scalar_type_error<T>(value: &ScalarRef<'_>) -> ErrorCode {
654+
ErrorCode::BadDataValueType(format!(
655+
"failed to downcast scalar {:?} into {}",
656+
value,
657+
std::any::type_name::<T>()
658+
))
659+
}
660+
661+
fn column_type_error<T>(value: &Column) -> ErrorCode {
662+
ErrorCode::BadDataValueType(format!(
663+
"failed to downcast column {:?} into {}",
664+
value,
665+
std::any::type_name::<T>()
666+
))
667+
}
668+
669+
fn domain_type_error<T>(value: &Domain) -> ErrorCode {
670+
ErrorCode::BadDataValueType(format!(
671+
"failed to downcast domain {:?} into {}",
672+
value,
673+
std::any::type_name::<T>()
674+
))
675+
}
676+
653677
/// [ValueType] includes the builder method of a data type based on [AccessType].
654678
pub trait ValueType: AccessType {
655679
type ColumnBuilder: Debug + Clone;

‎src/query/expression/src/types/any.rs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@
1515
use std::cmp::Ordering;
1616
use std::ops::Range;
1717

18+
use databend_common_exception::Result;
19+
20+
use super::AccessType;
21+
use super::BuilderMut;
22+
use super::DataType;
23+
use super::ValueType;
1824
use crate::property::Domain;
19-
use crate::types::AccessType;
20-
use crate::types::BuilderMut;
21-
use crate::types::DataType;
22-
use crate::types::ValueType;
2325
use crate::values::Column;
2426
use crate::values::Scalar;
2527
use crate::ColumnBuilder;
@@ -44,16 +46,16 @@ impl AccessType for AnyType {
4446
scalar.as_ref()
4547
}
4648

47-
fn try_downcast_scalar<'a>(scalar: &ScalarRef<'a>) -> Option<Self::ScalarRef<'a>> {
48-
Some(scalar.clone())
49+
fn try_downcast_scalar<'a>(scalar: &ScalarRef<'a>) -> Result<Self::ScalarRef<'a>> {
50+
Ok(scalar.clone())
4951
}
5052

51-
fn try_downcast_column(col: &Column) -> Option<Self::Column> {
52-
Some(col.clone())
53+
fn try_downcast_column(col: &Column) -> Result<Self::Column> {
54+
Ok(col.clone())
5355
}
5456

55-
fn try_downcast_domain(domain: &Domain) -> Option<Self::Domain> {
56-
Some(domain.clone())
57+
fn try_downcast_domain(domain: &Domain) -> Result<Self::Domain> {
58+
Ok(domain.clone())
5759
}
5860

5961
fn column_len(col: &Self::Column) -> usize {

‎src/query/expression/src/types/array.rs‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,21 @@ use databend_common_column::buffer::Buffer;
2222
use databend_common_exception::ErrorCode;
2323
use databend_common_exception::Result;
2424

25+
use super::column_type_error;
26+
use super::domain_type_error;
27+
use super::scalar_type_error;
28+
use super::AccessType;
2529
use super::AnyType;
30+
use super::ArgType;
31+
use super::BuilderExt;
32+
use super::Column;
33+
use super::DataType;
34+
use super::GenericMap;
2635
use super::ReturnType;
36+
use super::Scalar;
37+
use super::ScalarRef;
38+
use super::ValueType;
2739
use crate::property::Domain;
28-
use crate::types::AccessType;
29-
use crate::types::ArgType;
30-
use crate::types::BuilderExt;
31-
use crate::types::DataType;
32-
use crate::types::GenericMap;
33-
use crate::types::Scalar;
34-
use crate::types::ScalarRef;
35-
use crate::types::ValueType;
36-
use crate::values::Column;
3740
use crate::ColumnBuilder;
3841

3942
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -54,22 +57,25 @@ impl<T: AccessType> AccessType for ArrayType<T> {
5457
scalar.clone()
5558
}
5659

57-
fn try_downcast_scalar<'a>(scalar: &ScalarRef<'a>) -> Option<Self::ScalarRef<'a>> {
60+
fn try_downcast_scalar<'a>(scalar: &ScalarRef<'a>) -> Result<Self::ScalarRef<'a>> {
5861
match scalar {
5962
ScalarRef::Array(array) => T::try_downcast_column(array),
60-
_ => None,
63+
_ => Err(scalar_type_error::<Self>(scalar)),
6164
}
6265
}
6366

64-
fn try_downcast_column(col: &Column) -> Option<Self::Column> {
65-
ArrayColumn::try_downcast(col.as_array()?)
67+
fn try_downcast_column(col: &Column) -> Result<Self::Column> {
68+
let array = col
69+
.as_array()
70+
.ok_or_else(|| column_type_error::<Self>(col))?;
71+
ArrayColumn::try_downcast(array)
6672
}
6773

68-
fn try_downcast_domain(domain: &Domain) -> Option<Self::Domain> {
74+
fn try_downcast_domain(domain: &Domain) -> Result<Self::Domain> {
6975
match domain {
70-
Domain::Array(Some(domain)) => Some(Some(T::try_downcast_domain(domain)?)),
71-
Domain::Array(None) => Some(None),
72-
_ => None,
76+
Domain::Array(Some(domain)) => Ok(Some(T::try_downcast_domain(domain)?)),
77+
Domain::Array(None) => Ok(None),
78+
_ => Err(domain_type_error::<Self>(domain)),
7379
}
7480
}
7581

@@ -333,8 +339,8 @@ impl<T: ValueType> ArrayColumn<T> {
333339
}
334340

335341
impl ArrayColumn<AnyType> {
336-
pub fn try_downcast<T: AccessType>(&self) -> Option<ArrayColumn<T>> {
337-
Some(ArrayColumn {
342+
pub fn try_downcast<T: AccessType>(&self) -> Result<ArrayColumn<T>> {
343+
Ok(ArrayColumn {
338344
values: T::try_downcast_column(&self.values)?,
339345
offsets: self.offsets.clone(),
340346
})

‎src/query/expression/src/types/binary.rs‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,20 @@
1515
use std::cmp::Ordering;
1616
use std::ops::Range;
1717

18+
use databend_common_exception::Result;
19+
20+
use super::column_type_error;
21+
use super::domain_type_error;
22+
use super::scalar_type_error;
1823
use super::AccessType;
24+
use super::ArgType;
25+
use super::BuilderMut;
26+
use super::DataType;
27+
use super::GenericMap;
28+
use super::ReturnType;
29+
use super::ScalarRef;
30+
use super::ValueType;
1931
use crate::property::Domain;
20-
use crate::types::ArgType;
21-
use crate::types::BuilderMut;
22-
use crate::types::DataType;
23-
use crate::types::GenericMap;
24-
use crate::types::ReturnType;
25-
use crate::types::ScalarRef;
26-
use crate::types::ValueType;
2732
use crate::values::Column;
2833
use crate::values::Scalar;
2934
use crate::ColumnBuilder;
@@ -50,19 +55,24 @@ impl AccessType for BinaryType {
5055
scalar
5156
}
5257

53-
fn try_downcast_scalar<'a>(scalar: &ScalarRef<'a>) -> Option<Self::ScalarRef<'a>> {
54-
scalar.as_binary().cloned()
58+
fn try_downcast_scalar<'a>(scalar: &ScalarRef<'a>) -> Result<Self::ScalarRef<'a>> {
59+
scalar
60+
.as_binary()
61+
.cloned()
62+
.ok_or_else(|| scalar_type_error::<Self>(scalar))
5563
}
5664

57-
fn try_downcast_column(col: &Column) -> Option<Self::Column> {
58-
col.as_binary().cloned()
65+
fn try_downcast_column(col: &Column) -> Result<Self::Column> {
66+
col.as_binary()
67+
.cloned()
68+
.ok_or_else(|| column_type_error::<Self>(col))
5969
}
6070

61-
fn try_downcast_domain(domain: &Domain) -> Option<Self::Domain> {
71+
fn try_downcast_domain(domain: &Domain) -> Result<Self::Domain> {
6272
if domain.is_undefined() {
63-
Some(())
73+
Ok(())
6474
} else {
65-
None
75+
Err(domain_type_error::<Self>(domain))
6676
}
6777
}
6878

‎src/query/expression/src/types/bitmap.rs‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,22 @@
1515
use std::cmp::Ordering;
1616
use std::ops::Range;
1717

18+
use databend_common_exception::Result;
19+
1820
use super::binary::BinaryColumn;
1921
use super::binary::BinaryColumnBuilder;
2022
use super::binary::BinaryColumnIter;
23+
use super::column_type_error;
24+
use super::domain_type_error;
25+
use super::scalar_type_error;
26+
use super::AccessType;
27+
use super::ArgType;
28+
use super::BuilderMut;
29+
use super::DataType;
30+
use super::GenericMap;
31+
use super::ReturnType;
32+
use super::ValueType;
2133
use crate::property::Domain;
22-
use crate::types::AccessType;
23-
use crate::types::ArgType;
24-
use crate::types::BuilderMut;
25-
use crate::types::DataType;
26-
use crate::types::GenericMap;
27-
use crate::types::ReturnType;
28-
use crate::types::ValueType;
2934
use crate::values::Column;
3035
use crate::values::Scalar;
3136
use crate::ColumnBuilder;
@@ -49,19 +54,24 @@ impl AccessType for BitmapType {
4954
scalar
5055
}
5156

52-
fn try_downcast_scalar<'a>(scalar: &ScalarRef<'a>) -> Option<Self::ScalarRef<'a>> {
53-
scalar.as_bitmap().cloned()
57+
fn try_downcast_scalar<'a>(scalar: &ScalarRef<'a>) -> Result<Self::ScalarRef<'a>> {
58+
scalar
59+
.as_bitmap()
60+
.cloned()
61+
.ok_or_else(|| scalar_type_error::<Self>(scalar))
5462
}
5563

56-
fn try_downcast_column(col: &Column) -> Option<Self::Column> {
57-
col.as_bitmap().cloned()
64+
fn try_downcast_column(col: &Column) -> Result<Self::Column> {
65+
col.as_bitmap()
66+
.cloned()
67+
.ok_or_else(|| column_type_error::<Self>(col))
5868
}
5969

60-
fn try_downcast_domain(domain: &Domain) -> Option<Self::Domain> {
70+
fn try_downcast_domain(domain: &Domain) -> Result<Self::Domain> {
6171
if domain.is_undefined() {
62-
Some(())
72+
Ok(())
6373
} else {
64-
None
74+
Err(domain_type_error::<Self>(domain))
6575
}
6676
}
6777

0 commit comments

Comments
 (0)