Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Rename TypeSignature::NullAry --> TypeSignature::Nullary and improve comments #13817

Merged
merged 5 commits into from
Dec 19, 2024
Merged
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
82 changes: 57 additions & 25 deletions datafusion/expr-common/src/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ pub enum Volatility {
/// adds a cast such as `cos(CAST int_column AS DOUBLE)` during planning.
///
/// # Data Types
///
/// ## Timestamps
findepi marked this conversation as resolved.
Show resolved Hide resolved
///
/// Types to match are represented using Arrow's [`DataType`]. [`DataType::Timestamp`] has an optional variable
/// timezone specification. To specify a function can handle a timestamp with *ANY* timezone, use
/// the [`TIMEZONE_WILDCARD`]. For example:
Expand All @@ -93,60 +96,89 @@ pub enum Volatility {
pub enum TypeSignature {
/// One or more arguments of a common type out of a list of valid types.
///
/// For functions that take no arguments (e.g. `random()` see [`TypeSignature::Nullary`]).
///
/// # Examples
/// A function such as `concat` is `Variadic(vec![DataType::Utf8, DataType::LargeUtf8])`
///
/// A function such as `concat` is `Variadic(vec![DataType::Utf8,
/// DataType::LargeUtf8])`
Comment on lines +103 to +104
Copy link
Member

Choose a reason for hiding this comment

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

nit:
Maybe it would be more readable to go without DataType: for type names?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in #13840

Variadic(Vec<DataType>),
/// The acceptable signature and coercions rules to coerce arguments to this
/// signature are special for this function. If this signature is specified,
/// DataFusion will call `ScalarUDFImpl::coerce_types` to prepare argument types.
UserDefined,
/// One or more arguments with arbitrary types
VariadicAny,
/// Fixed number of arguments of an arbitrary but equal type out of a list of valid types.
/// One or more arguments of an arbitrary but equal type out of a list of valid types.
///
/// # Examples
///
/// 1. A function of one argument of f64 is `Uniform(1, vec![DataType::Float64])`
/// 2. A function of one argument of f64 or f32 is `Uniform(1, vec![DataType::Float32, DataType::Float64])`
Uniform(usize, Vec<DataType>),
/// Exact number of arguments of an exact type
/// One or more arguments with exactly the specified types in order.
///
/// For functions that take no arguments (e.g. `random()`) use [`TypeSignature::Nullary`].
Exact(Vec<DataType>),
/// The number of arguments that can be coerced to in order
/// One or more arguments belonging to the [`TypeSignatureClass`], in order.
///
/// For example, `Coercible(vec![logical_float64()])` accepts
/// arguments like `vec![DataType::Int32]` or `vec![DataType::Float32]`
/// since i32 and f32 can be casted to f64
/// since i32 and f32 can be cast to f64
///
/// For functions that take no arguments (e.g. `random()`) see [`TypeSignature::Nullary`].
Coercible(Vec<TypeSignatureClass>),
/// The arguments will be coerced to a single type based on the comparison rules.
/// For example, i32 and i64 has coerced type Int64.
/// One or more arguments that can be "compared"
Copy link
Member

Choose a reason for hiding this comment

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

what does this mean ?

Copy link
Member

Choose a reason for hiding this comment

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

this needs a follow-up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed -- this can be made better. I will attempt a follow up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

///
/// Each argument will be coerced to a single type based on comparison rules.
/// For example a function called with `i32` and `i64` has coerced type `Int64` so
/// each argument will be coerced to `Int64` before the function is invoked.
///
/// Note:
/// - If compares with numeric and string, numeric is preferred for numeric string cases. For example, nullif('2', 1) has coerced types Int64.
/// - If compares with numeric and string, numeric is preferred for numeric string cases. For example, `nullif('2', 1)` has coerced types `Int64`.
Copy link
Member

Choose a reason for hiding this comment

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

is the reader expected to know/assume that nullif uses signature "Comparable"?

Copy link
Contributor

Choose a reason for hiding this comment

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

actual example direct reader to understand this easily

/// - If the result is Null, it will be coerced to String (Utf8View).
/// - See [`comparison_coercion`] for more details.
/// - For functions that take no arguments (e.g. `random()` see [`TypeSignature::Nullary`]).
///
/// See `comparison_coercion_numeric` for more details.
/// [`comparison_coercion`]: crate::type_coercion::binary::comparison_coercion
Comparable(usize),
/// Fixed number of arguments of arbitrary types, number should be larger than 0
/// One or more arguments of arbitrary types.
///
/// For functions that take no arguments (e.g. `random()`) use [`TypeSignature::Nullary`].
Any(usize),
/// Matches exactly one of a list of [`TypeSignature`]s. Coercion is attempted to match
/// the signatures in order, and stops after the first success, if any.
/// Matches exactly one of a list of [`TypeSignature`]s.
///
/// Coercion is attempted to match the signatures in order, and stops after
/// the first success, if any.
///
/// # Examples
/// Function `make_array` takes 0 or more arguments with arbitrary types, its `TypeSignature`
///
/// Since `make_array` takes 0 or more arguments with arbitrary types, its `TypeSignature`
/// is `OneOf(vec![Any(0), VariadicAny])`.
findepi marked this conversation as resolved.
Show resolved Hide resolved
OneOf(Vec<TypeSignature>),
/// Specifies Signatures for array functions
/// A function that has an [`ArrayFunctionSignature`]
ArraySignature(ArrayFunctionSignature),
/// Fixed number of arguments of numeric types.
/// One or more arguments of numeric types.
///
/// See [`NativeType::is_numeric`] to know which type is considered numeric
///
/// [`NativeType::is_numeric`]: datafusion_common
/// For functions that take no arguments (e.g. `random()`) use [`TypeSignature::Nullary`].
///
/// [`NativeType::is_numeric`]: datafusion_common::types::NativeType::is_numeric
Numeric(usize),
/// Fixed number of arguments of all the same string types.
/// One or arguments of all the same string types.
///
/// The precedence of type from high to low is Utf8View, LargeUtf8 and Utf8.
/// Null is considered as `Utf8` by default
/// Dictionary with string value type is also handled.
///
/// For example, if a function is called with (utf8, large_utf8), all
/// arguments will be coerced to `LargeUtf8`
///
/// For functions that take no arguments (e.g. `random()` use [`TypeSignature::Nullary`]).
String(usize),
/// Zero argument
NullAry,
/// No arguments
Nullary,
}

impl TypeSignature {
Expand Down Expand Up @@ -243,7 +275,7 @@ impl Display for ArrayFunctionSignature {
impl TypeSignature {
pub fn to_string_repr(&self) -> Vec<String> {
match self {
TypeSignature::NullAry => {
TypeSignature::Nullary => {
vec!["NullAry()".to_string()]
}
TypeSignature::Variadic(types) => {
Expand Down Expand Up @@ -302,7 +334,7 @@ impl TypeSignature {
pub fn supports_zero_argument(&self) -> bool {
match &self {
TypeSignature::Exact(vec) => vec.is_empty(),
TypeSignature::NullAry => true,
TypeSignature::Nullary => true,
TypeSignature::OneOf(types) => types
.iter()
.any(|type_sig| type_sig.supports_zero_argument()),
Expand Down Expand Up @@ -368,7 +400,7 @@ impl TypeSignature {
// TODO: Implement for other types
TypeSignature::Any(_)
| TypeSignature::Comparable(_)
| TypeSignature::NullAry
| TypeSignature::Nullary
| TypeSignature::VariadicAny
| TypeSignature::ArraySignature(_)
| TypeSignature::UserDefined => vec![],
Expand Down Expand Up @@ -502,7 +534,7 @@ impl Signature {

pub fn nullary(volatility: Volatility) -> Self {
Signature {
type_signature: TypeSignature::NullAry,
type_signature: TypeSignature::Nullary,
volatility,
}
}
Expand Down Expand Up @@ -579,10 +611,10 @@ mod tests {
TypeSignature::Exact(vec![]),
TypeSignature::OneOf(vec![
TypeSignature::Exact(vec![DataType::Int8]),
TypeSignature::NullAry,
TypeSignature::Nullary,
TypeSignature::Uniform(1, vec![DataType::Int8]),
]),
TypeSignature::NullAry,
TypeSignature::Nullary,
];

for case in positive_cases {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/expr-common/src/type_coercion/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ pub fn comparison_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<D
.or_else(|| struct_coercion(lhs_type, rhs_type))
}

// Similar to comparison_coercion but prefer numeric if compares with numeric and string
/// Similar to [`comparison_coercion`] but prefer numeric if compares with numeric and string
pub fn comparison_coercion_numeric(
lhs_type: &DataType,
rhs_type: &DataType,
Expand Down
4 changes: 2 additions & 2 deletions datafusion/expr/src/type_coercion/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ fn is_well_supported_signature(type_signature: &TypeSignature) -> bool {
| TypeSignature::String(_)
| TypeSignature::Coercible(_)
| TypeSignature::Any(_)
| TypeSignature::NullAry
| TypeSignature::Nullary
| TypeSignature::Comparable(_)
)
}
Expand Down Expand Up @@ -715,7 +715,7 @@ fn get_valid_types(
}
}
},
TypeSignature::NullAry => {
TypeSignature::Nullary => {
if !current_types.is_empty() {
return plan_err!(
"The function expected zero argument but received {}",
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-aggregate/src/count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl Count {
pub fn new() -> Self {
Self {
signature: Signature::one_of(
vec![TypeSignature::VariadicAny, TypeSignature::NullAry],
vec![TypeSignature::VariadicAny, TypeSignature::Nullary],
Volatility::Immutable,
),
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-nested/src/make_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl MakeArray {
pub fn new() -> Self {
Self {
signature: Signature::one_of(
vec![TypeSignature::NullAry, TypeSignature::UserDefined],
vec![TypeSignature::Nullary, TypeSignature::UserDefined],
Volatility::Immutable,
),
aliases: vec![String::from("make_list")],
Expand Down
Loading