jayzhan211 merged PR #14532:
URL: https://github.com/apache/datafusion/pull/14532
--
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...@dat
jayzhan211 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2659324140
Thanks @jkosh44
--
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 comme
jkosh44 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2656959911
I think this might also be an API change? I don't have the permissions to
add the tag though.
--
This is an automated message from the Apache Git Service.
To respond to the message
jkosh44 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2656668319
Thanks for the review @jayzhan211 and for all of your helpful feedback!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitH
jayzhan211 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2656285292
`array_element` doesn't change the list so not coerce to list makes sense to
me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please l
jkosh44 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2655302737
> It might because of `array_coercion` you set for the function is incorrect
🤔
The query that fails is `array.slt:1221` which is
```
query IT
select array_element(arro
jayzhan211 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2655163076
> > It is because of this, I think we now only coerce to list if the flag is
set
>
> Are you saying that the function should look something like this?
>
> ```rust
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1952948300
##
datafusion/functions-nested/src/array_has.rs:
##
@@ -94,7 +94,7 @@ impl Default for ArrayHas {
impl ArrayHas {
pub fn new() -> Self {
Self {
-
jkosh44 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2654129469
> It is because of this, I think we now only coerce to list if the flag is
set
Are you saying that the function should look something like this?
```Rust
fn array(
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1952912279
##
datafusion/expr-common/src/signature.rs:
##
@@ -286,6 +258,72 @@ impl Display for ArrayFunctionSignature {
}
}
+/// A wrapper around a vec of [`ArrayFun
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1951785203
##
datafusion/functions-nested/src/array_has.rs:
##
@@ -94,7 +94,7 @@ impl Default for ArrayHas {
impl ArrayHas {
pub fn new() -> Self {
Self {
-
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1951783336
##
datafusion/functions-nested/src/array_has.rs:
##
@@ -94,7 +94,7 @@ impl Default for ArrayHas {
impl ArrayHas {
pub fn new() -> Self {
Self {
-
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1951778634
##
datafusion/expr-common/src/signature.rs:
##
@@ -286,6 +258,72 @@ impl Display for ArrayFunctionSignature {
}
}
+/// A wrapper around a vec of [`Array
jayzhan211 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2652335822
> get_valid_types() always coerces the outermost FixedSizeList to List, no
matter what the value of the flag is. I'm pretty sure that matches existing
behavior, but I don't fully
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1951358636
##
datafusion/functions-nested/src/remove.rs:
##
@@ -98,7 +99,7 @@ impl ScalarUDFImpl for ArrayRemove {
}
fn return_type(&self, arg_types: &[DataType])
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1951358636
##
datafusion/functions-nested/src/remove.rs:
##
@@ -98,7 +99,7 @@ impl ScalarUDFImpl for ArrayRemove {
}
fn return_type(&self, arg_types: &[DataType])
jkosh44 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2651638551
@jayzhan211 I just pushed a commit with your idea about adding a flag for
array coercion. I'm feeling pretty good about the state of this PR, but I have
the following two open questi
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950526095
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950529828
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950526095
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950529828
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950529828
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950526095
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950225082
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Has
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950125583
##
datafusion/functions-nested/src/remove.rs:
##
@@ -98,7 +99,7 @@ impl ScalarUDFImpl for ArrayRemove {
}
fn return_type(&self, arg_types: &[DataTyp
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950111843
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950111843
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1950111843
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1949114839
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Has
jkosh44 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2648300594
> It might be a better approach to not modify the accepted argument types
(i.e. don't convert `FixedSizeList` to `List` in `get_valid_types()`, and
instead move the logic to `return_
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1949114839
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Has
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1949116013
##
datafusion/common/src/utils/mod.rs:
##
@@ -602,26 +602,46 @@ pub fn base_type(data_type: &DataType) -> DataType {
///
/// let data_type =
DataType::List(Arc:
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(A
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1948299517
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jayzhan211 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1948299517
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd,
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1947908046
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,11 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Has
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1946590323
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,13 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Has
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1946583823
##
datafusion/common/src/utils/mod.rs:
##
@@ -602,26 +602,46 @@ pub fn base_type(data_type: &DataType) -> DataType {
///
/// let data_type =
DataType::List(Arc:
jkosh44 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2641969506
> See this, [#13819
(comment)](https://github.com/apache/datafusion/issues/13819#issuecomment-2552554818).
We only convert an inner fixed-size list to a regular list when the functio
jayzhan211 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2641979167
> So it sounds like we need to include in the array signature whether or not
the function might change the size of the list and use that information.
yes
--
This is an au
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1945901967
##
datafusion/expr-common/src/signature.rs:
##
@@ -286,6 +261,34 @@ impl Display for ArrayFunctionSignature {
}
}
+#[derive(Debug, Clone, PartialEq, Eq, Pa
jkosh44 commented on code in PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#discussion_r1945901504
##
datafusion/expr-common/src/signature.rs:
##
@@ -227,25 +226,11 @@ impl Display for TypeSignatureClass {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Has
jayzhan211 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2641836978
See this,
https://github.com/apache/datafusion/issues/13819#issuecomment-2552554818.
We only convert an inner fixed-size list to a regular list when the function
performs a mut
jkosh44 commented on PR #14532:
URL: https://github.com/apache/datafusion/pull/14532#issuecomment-2641819030
This is failing CI for the following reason.
Previously, in `get_valid_types()` functions with the array signature
`ArrayAndIndexes` and `Array` would convert top level `Fixed
jkosh44 opened a new pull request, #14532:
URL: https://github.com/apache/datafusion/pull/14532
This commit allows for more expressive array function signatures.
Previously, `ArrayFunctionSignature` was an enum of potential argument
combinations and orders. For many array functions, none of
45 matches
Mail list logo
