Skip to content

refactor!: consistent null handling in coercible signatures #15404

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions datafusion/expr-common/src/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,6 @@ impl TypeSignatureClass {
self: &TypeSignatureClass,
logical_type: &NativeType,
) -> bool {
if logical_type == &NativeType::Null {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description says

Are there any user-facing changes?
No.

However, doesn't this change means that existing TypeSignature::Coercable that handle Nulls will need to add Coercion::new_implicit to retain the same behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's right, I forgot about that. PR description updated.

return true;
}

match self {
TypeSignatureClass::Native(t) if t.native() == logical_type => true,
TypeSignatureClass::Timestamp if logical_type.is_timestamp() => true,
Expand Down
14 changes: 11 additions & 3 deletions datafusion/functions-nested/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use arrow::datatypes::DataType::{
};
use datafusion_common::cast::{as_large_list_array, as_list_array};
use datafusion_common::exec_err;
use datafusion_common::types::logical_string;
use datafusion_common::types::{logical_null, logical_string, NativeType};
use datafusion_expr::{
Coercion, ColumnarValue, Documentation, ScalarUDFImpl, Signature, TypeSignature,
TypeSignatureClass, Volatility,
Expand Down Expand Up @@ -255,11 +255,19 @@ impl StringToArray {
vec![
TypeSignature::Coercible(vec![
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
]),
TypeSignature::Coercible(vec![
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
]),
],
Expand Down
26 changes: 21 additions & 5 deletions datafusion/functions/src/crypto/digest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use super::basic::{digest, utf8_or_binary_to_binary_type};
use arrow::datatypes::DataType;
use datafusion_common::{
types::{logical_binary, logical_string},
types::{logical_binary, logical_null, logical_string, NativeType},
Result,
};
use datafusion_expr::{
Expand Down Expand Up @@ -72,12 +72,28 @@ impl DigestFunc {
signature: Signature::one_of(
vec![
TypeSignature::Coercible(vec![
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
]),
TypeSignature::Coercible(vec![
Coercion::new_exact(TypeSignatureClass::Native(logical_binary())),
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::Binary,
),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
]),
],
Volatility::Immutable,
Expand Down
12 changes: 9 additions & 3 deletions datafusion/functions/src/crypto/md5.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::crypto::basic::md5;
use arrow::datatypes::DataType;
use datafusion_common::{
plan_err,
types::{logical_binary, logical_string, NativeType},
types::{logical_binary, logical_null, logical_string, NativeType},
Result,
};
use datafusion_expr::{
Expand Down Expand Up @@ -62,12 +62,18 @@ impl Md5Func {
vec![
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_string())],
vec![
TypeSignatureClass::Native(logical_string()),
TypeSignatureClass::Native(logical_null()),
],
NativeType::String,
)]),
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_binary())],
vec![
TypeSignatureClass::Native(logical_binary()),
TypeSignatureClass::Native(logical_null()),
],
NativeType::Binary,
)]),
],
Expand Down
12 changes: 9 additions & 3 deletions datafusion/functions/src/crypto/sha224.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use super::basic::{sha224, utf8_or_binary_to_binary_type};
use arrow::datatypes::DataType;
use datafusion_common::{
types::{logical_binary, logical_string, NativeType},
types::{logical_binary, logical_null, logical_string, NativeType},
Result,
};
use datafusion_expr::{
Expand Down Expand Up @@ -62,12 +62,18 @@ impl SHA224Func {
vec![
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_string())],
vec![
TypeSignatureClass::Native(logical_string()),
TypeSignatureClass::Native(logical_null()),
],
NativeType::String,
)]),
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_binary())],
vec![
TypeSignatureClass::Native(logical_binary()),
TypeSignatureClass::Native(logical_null()),
],
NativeType::Binary,
)]),
],
Expand Down
12 changes: 9 additions & 3 deletions datafusion/functions/src/crypto/sha256.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use super::basic::{sha256, utf8_or_binary_to_binary_type};
use arrow::datatypes::DataType;
use datafusion_common::{
types::{logical_binary, logical_string, NativeType},
types::{logical_binary, logical_null, logical_string, NativeType},
Result,
};
use datafusion_expr::{
Expand Down Expand Up @@ -61,12 +61,18 @@ impl SHA256Func {
vec![
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_string())],
vec![
TypeSignatureClass::Native(logical_string()),
TypeSignatureClass::Native(logical_null()),
],
NativeType::String,
)]),
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_binary())],
vec![
TypeSignatureClass::Native(logical_binary()),
TypeSignatureClass::Native(logical_null()),
],
NativeType::Binary,
)]),
],
Expand Down
12 changes: 9 additions & 3 deletions datafusion/functions/src/crypto/sha384.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use super::basic::{sha384, utf8_or_binary_to_binary_type};
use arrow::datatypes::DataType;
use datafusion_common::{
types::{logical_binary, logical_string, NativeType},
types::{logical_binary, logical_null, logical_string, NativeType},
Result,
};
use datafusion_expr::{
Expand Down Expand Up @@ -61,12 +61,18 @@ impl SHA384Func {
vec![
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_string())],
vec![
TypeSignatureClass::Native(logical_string()),
TypeSignatureClass::Native(logical_null()),
],
NativeType::String,
)]),
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_binary())],
vec![
TypeSignatureClass::Native(logical_binary()),
TypeSignatureClass::Native(logical_null()),
],
NativeType::Binary,
)]),
],
Expand Down
12 changes: 9 additions & 3 deletions datafusion/functions/src/crypto/sha512.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use super::basic::{sha512, utf8_or_binary_to_binary_type};
use arrow::datatypes::DataType;
use datafusion_common::{
types::{logical_binary, logical_string, NativeType},
types::{logical_binary, logical_null, logical_string, NativeType},
Result,
};
use datafusion_expr::{
Expand Down Expand Up @@ -61,12 +61,18 @@ impl SHA512Func {
vec![
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_string())],
vec![
TypeSignatureClass::Native(logical_string()),
TypeSignatureClass::Native(logical_null()),
],
NativeType::String,
)]),
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_binary()),
vec![TypeSignatureClass::Native(logical_binary())],
vec![
TypeSignatureClass::Native(logical_binary()),
TypeSignatureClass::Native(logical_null()),
],
NativeType::Binary,
)]),
],
Expand Down
26 changes: 21 additions & 5 deletions datafusion/functions/src/regex/regexplike.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use arrow::array::{Array, ArrayRef, AsArray, GenericStringArray};
use arrow::compute::kernels::regexp;
use arrow::datatypes::DataType;
use arrow::datatypes::DataType::{LargeUtf8, Utf8, Utf8View};
use datafusion_common::types::logical_string;
use datafusion_common::types::{logical_null, logical_string, NativeType};
use datafusion_common::{
arrow_datafusion_err, exec_err, internal_err, plan_err, DataFusionError, Result,
ScalarValue,
Expand Down Expand Up @@ -84,12 +84,28 @@ impl RegexpLikeFunc {
signature: Signature::one_of(
vec![
TypeSignature::Coercible(vec![
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
]),
TypeSignature::Coercible(vec![
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
]),
],
Expand Down
10 changes: 6 additions & 4 deletions datafusion/functions/src/string/ascii.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::utils::make_scalar_function;
use arrow::array::{ArrayAccessor, ArrayIter, ArrayRef, AsArray, Int32Array};
use arrow::datatypes::DataType;
use arrow::error::ArrowError;
use datafusion_common::types::logical_string;
use datafusion_common::types::{logical_null, logical_string, NativeType};
use datafusion_common::{internal_err, Result};
use datafusion_expr::{ColumnarValue, Documentation, TypeSignatureClass};
use datafusion_expr::{ScalarFunctionArgs, ScalarUDFImpl, Signature, Volatility};
Expand Down Expand Up @@ -64,9 +64,11 @@ impl AsciiFunc {
pub fn new() -> Self {
Self {
signature: Signature::coercible(
vec![Coercion::new_exact(TypeSignatureClass::Native(
logical_string(),
))],
vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
)],
Volatility::Immutable,
),
}
Expand Down
10 changes: 6 additions & 4 deletions datafusion/functions/src/string/bit_length.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use arrow::datatypes::DataType;
use std::any::Any;

use crate::utils::utf8_to_int_type;
use datafusion_common::types::logical_string;
use datafusion_common::types::{logical_null, logical_string, NativeType};
use datafusion_common::utils::take_function_args;
use datafusion_common::{Result, ScalarValue};
use datafusion_expr::{
Expand Down Expand Up @@ -60,9 +60,11 @@ impl BitLengthFunc {
pub fn new() -> Self {
Self {
signature: Signature::coercible(
vec![Coercion::new_exact(TypeSignatureClass::Native(
logical_string(),
))],
vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
)],
Volatility::Immutable,
),
}
Expand Down
18 changes: 14 additions & 4 deletions datafusion/functions/src/string/btrim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::string::common::*;
use crate::utils::{make_scalar_function, utf8_to_str_type};
use arrow::array::{ArrayRef, OffsetSizeTrait};
use arrow::datatypes::DataType;
use datafusion_common::types::logical_string;
use datafusion_common::types::{logical_null, logical_string, NativeType};
use datafusion_common::{exec_err, Result};
use datafusion_expr::function::Hint;
use datafusion_expr::{
Expand Down Expand Up @@ -83,11 +83,21 @@ impl BTrimFunc {
signature: Signature::one_of(
vec![
TypeSignature::Coercible(vec![
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
]),
TypeSignature::Coercible(vec![Coercion::new_exact(
TypeSignature::Coercible(vec![Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
)]),
],
Volatility::Immutable,
Expand Down
8 changes: 6 additions & 2 deletions datafusion/functions/src/string/contains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use arrow::array::{Array, ArrayRef, AsArray};
use arrow::compute::contains as arrow_contains;
use arrow::datatypes::DataType;
use arrow::datatypes::DataType::{Boolean, LargeUtf8, Utf8, Utf8View};
use datafusion_common::types::logical_string;
use datafusion_common::types::{logical_null, logical_string, NativeType};
use datafusion_common::{exec_err, DataFusionError, Result};
use datafusion_expr::binary::{binary_to_string_coercion, string_coercion};
use datafusion_expr::{
Expand Down Expand Up @@ -63,7 +63,11 @@ impl ContainsFunc {
signature: Signature::coercible(
vec![
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_exact(TypeSignatureClass::Native(logical_string())),
Coercion::new_implicit(
TypeSignatureClass::Native(logical_string()),
vec![TypeSignatureClass::Native(logical_null())],
NativeType::String,
),
],
Volatility::Immutable,
),
Expand Down
Loading