findepi commented on code in PR #13817:
URL: https://github.com/apache/datafusion/pull/13817#discussion_r1889223428


##########
datafusion/expr-common/src/signature.rs:
##########
@@ -102,51 +106,75 @@ pub enum TypeSignature {
     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()` see 
[`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"
+    ///
+    /// 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`.
     /// - 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()` see 
[`TypeSignature::Nullary`]).

Review Comment:
   ```suggestion
       /// For functions that take no arguments (e.g. `random()`) use 
[`TypeSignature::Nullary`].
   ```
   
   



##########
datafusion/expr-common/src/signature.rs:
##########
@@ -102,51 +106,75 @@ pub enum TypeSignature {
     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()` see 
[`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"

Review Comment:
   what does this mean ?



##########
datafusion/expr-common/src/signature.rs:
##########
@@ -102,51 +106,75 @@ pub enum TypeSignature {
     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()` see 
[`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"
+    ///
+    /// 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`.

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



##########
datafusion/expr-common/src/signature.rs:
##########
@@ -102,51 +106,75 @@ pub enum TypeSignature {
     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()` see 
[`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"
+    ///
+    /// 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`.
     /// - 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()` see 
[`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])`.

Review Comment:
   This IMO is wrong design of those signatures, but whatever (for now)



##########
datafusion/expr-common/src/signature.rs:
##########
@@ -102,51 +106,75 @@ pub enum TypeSignature {
     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()` see 
[`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"
+    ///
+    /// 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`.
     /// - 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()` see 
[`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])`.
     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()` see 
[`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()` see 
[`TypeSignature::Nullary`]).

Review Comment:
   here too



##########
datafusion/expr-common/src/signature.rs:
##########
@@ -102,51 +106,75 @@ pub enum TypeSignature {
     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()` see 
[`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"
+    ///
+    /// 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`.
     /// - 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()` see 
[`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])`.
     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()` see 
[`TypeSignature::Nullary`]).

Review Comment:
   ```suggestion
       /// For functions that take no arguments (e.g. `random()`) use 
[`TypeSignature::Nullary`].
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to