You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "thisisnic (via GitHub)" <gi...@apache.org> on 2023/02/27 14:32:38 UTC
[GitHub] [arrow] thisisnic opened a new issue, #34365: [C++] Substrait cast expression fails on input types other than field reference
thisisnic opened a new issue, #34365:
URL: https://github.com/apache/arrow/issues/34365
### Describe the bug, including details regarding any error messages, version, and platform.
The Substrait cast function has an `input` argument which takes an `Expression`; however, the current implementation only work with Expressions which are Field References and not any other types, i.e. the following plan does not run (see https://github.com/ibis-project/ibis-substrait/pull/414)
```
{
"extensionUris": [
{
"extensionUriAnchor": 1
}
],
"extensions": [
{
"extensionFunction": {
"extensionUriReference": 1,
"functionAnchor": 1,
"name": "floor"
}
}
],
"relations": [
{
"root": {
"input": {
"project": {
"common": {
"emit": {
"outputMapping": [
1
]
}
},
"input": {
"read": {
"common": {
"direct": {}
},
"baseSchema": {
"names": [
"a"
],
"struct": {
"types": [
{
"fp64": {
"nullability": "NULLABILITY_NULLABLE"
}
}
],
"nullability": "NULLABILITY_REQUIRED"
}
},
"namedTable": {
"names": [
"table0"
]
}
}
},
"expressions": [
{
"cast": {
"type": {
"i64": {
"nullability": "NULLABILITY_NULLABLE"
}
},
"input": {
"scalarFunction": {
"functionReference": 1,
"outputType": {
"i64": {
"nullability": "NULLABILITY_NULLABLE"
}
}
}
}
}
}
]
}
},
"names": [
"a"
]
}
}
]
}
```
### Component(s)
C++
--
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: issues-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] thisisnic commented on issue #34365: [C++] Substrait cast expression fails on input types other than field reference
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on issue #34365:
URL: https://github.com/apache/arrow/issues/34365#issuecomment-1446440310
I think we need to update the code which translates from a Substrait plan to Acero code to check the type of the expression which is passed in, and proceed as-is if it's a Literal or Field Reference, but do something else if it's a ScalarFunction? I'm not sure exactly what. CC @westonpace
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] westonpace commented on issue #34365: [C++] Substrait cast expression fails on input types other than field reference
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on issue #34365:
URL: https://github.com/apache/arrow/issues/34365#issuecomment-1446685564
For most nodes, whether an expression input can be "anything" or "only a direct reference" has been described to me as a "logical" vs. "physical" thing. You can always convert to a plan where the input to a cast is a direct reference by introducing a project node. In other words:
```
project:
exprs:
- add(2, cast(tolower("x"), int32()))
names:
- "out"
input:
src
```
can become
```
project:
exprs:
- add(2, cast("x_lower", int32()))
names:
- "out"
input:
project:
exprs:
- tolower("x")
names:
- "x_lower"
input:
src
```
There are other cases where Substrait's existing nodes are "too logical" for Substrait and we are a little restrictive. For example, in the join node and the aggregate node. We call out this caveat here: https://arrow.apache.org/docs/dev/cpp/streaming_execution.html#expressions-general
That being said, `cast` is just a function call in Acero and we do have the ability for functions to take other functions as input. So it does seem like this is one place where we don't have to be quite so restrictive.
In the linked issue you mentioned:
> The Acero cast function looks like it can either take an Array or an individual object of class Datum (so a scalar, array, etc).
This is true for the C++ cast function. However, this is not true for "expressions". In other words, a `compute::call` is constructed as follows:
```
ARROW_EXPORT
Expression call(std::string function, std::vector<Expression> arguments,
std::shared_ptr<FunctionOptions> options = NULLPTR);
```
So it can receive any `compute::Expression` as an argument. This discrepancy is handled during "expression execution" (`compute::ExecuteScalarExpression`). In expression evaluation we travel the AST and convert each of the arguments into an array by executing the sub-expressions. Finally, these input arrays are passed to the actual function call.
So...I'm not sure why this isn't working. What is the error?
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] westonpace closed issue #34365: [C++] Substrait cast expression fails on input types other than field reference
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace closed issue #34365: [C++] Substrait cast expression fails on input types other than field reference
URL: https://github.com/apache/arrow/issues/34365
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] icexelloss commented on issue #34365: [C++] Substrait cast expression fails on input types other than field reference
Posted by "icexelloss (via GitHub)" <gi...@apache.org>.
icexelloss commented on issue #34365:
URL: https://github.com/apache/arrow/issues/34365#issuecomment-1446622462
```
"expressions": [
{
"cast": {
"type": {
"i64": {
"nullability": "NULLABILITY_NULLABLE"
}
},
"input": {
"scalarFunction": {
"functionReference": 1,
"outputType": {
"i64": {
"nullability": "NULLABILITY_NULLABLE"
}
}
}
}
}
}
]
```
This doesn't quite make sense to me. This is saying "cast the output of scalar function 1 (floor) to int" but it doesn't say what is the input to the floor function (also doesn't seem to appear anywhere else in the plan what is the input to "floor")
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] westonpace commented on issue #34365: [C++] Substrait cast expression fails on input types other than field reference
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on issue #34365:
URL: https://github.com/apache/arrow/issues/34365#issuecomment-1613980004
I'm closing this as I don't think any more work is being done on this currently and it isn't clear anything is wrong. We can reopen later if needed.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org