jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1948299765
##########
datafusion/common/src/utils/mod.rs:
##########
@@ -602,26 +602,46 @@ pub fn base_type(data_type: &DataType) -> DataType {
///
/// let data_type =
DataType::List(Arc::new(Field::new_list_field(DataType::Int32, true)));
/// let base_type = DataType::Float64;
-/// let coerced_type = coerced_type_with_base_type_only(&data_type,
&base_type);
+/// let coerced_type = coerced_type_with_base_type_only(&data_type,
&base_type, true);
/// assert_eq!(coerced_type,
DataType::List(Arc::new(Field::new_list_field(DataType::Float64, true))));
pub fn coerced_type_with_base_type_only(
data_type: &DataType,
base_type: &DataType,
+ mutable: bool,
Review Comment:
Then it means the enum should be move to here.
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)]
pub enum ArrayFunctionSignature {
- /// Specialized Signature for ArrayAppend and similar functions
- /// The first argument should be List/LargeList/FixedSizedList, and the
second argument should be non-list or list.
- /// The second argument's list dimension should be one dimension less than
the first argument's list dimension.
- /// List dimension of the List/LargeList is equivalent to the number of
List.
- /// List dimension of the non-list is 0.
- ArrayAndElement,
- /// Specialized Signature for ArrayPrepend and similar functions
- /// The first argument should be non-list or list, and the second argument
should be List/LargeList.
- /// The first argument's list dimension should be one dimension less than
the second argument's list dimension.
- ElementAndArray,
- /// Specialized Signature for Array functions of the form (List/LargeList,
Index+)
- /// The first argument should be List/LargeList/FixedSizedList, and the
next n arguments should be Int64.
- ArrayAndIndexes(NonZeroUsize),
- /// Specialized Signature for Array functions of the form (List/LargeList,
Element, Optional Index)
- ArrayAndElementAndOptionalIndex,
- /// Specialized Signature for ArrayEmpty and similar functions
- /// The function takes a single argument that must be a
List/LargeList/FixedSizeList
- /// or something that can be coerced to one of those types.
- Array,
+ /// A function takes at least one List/LargeList/FixedSizeList argument.
+ Array {
+ /// A full list of the arguments accepted by this function.
+ arguments: Vec<ArrayFunctionArgument>,
+ /// Whether any of the input arrays are modified.
+ mutability: ArrayFunctionMutability,
Review Comment:
> Why does the mutability of a function affect what types are accepted as
arguments? It seems like the mutability of the function should affect the
return types of the function not the argument types.
It looks like the current code behaves as expected: both Fixed and List are
accepted regardless of the specified mutability, and the return type is
determined by the mutability setting.
Instead of modeling "mutability", how about we define the desired type
explicitly with signature. It can be either List or FixedSizeList, so we coerce
to that desired type
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
