You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2025/01/27 18:28:29 UTC
[I] [EPIC] Improve supported data type checks and tests for expressions [datafusion-comet]
andygrove opened a new issue, #1345:
URL: https://github.com/apache/datafusion-comet/issues/1345
### What is the problem the feature request solves?
## Problem Statement
When translating Spark expressions to protocol buffer-encoded native expressions in `QueryPlanSerde`, we should check that we can support the expressions natively and guarantee that we will produce the same results as Spark.
One of these checks should be to check that we support the expression for the specified input data types. For example, maybe we can support `Sha2` with string inputs but not with binary inputs (this is just a hypothetical example).
We should ensure that we have tests to cover all of the data types that we say that we can support.
We currently have some gaps in this area and this epic is intended to link to issues to improve the situation.
Here are some specific examples where our current approach is lacking.
## Shared data type checks for multiple expressions
In some cases, we call a generic `supportedDataType` method:
```scala
case add @ Add(left, right, _) if supportedDataType(left.dataType) =>
case s @ execution.ScalarSubquery(_, _) if supportedDataType(s.dataType) =>
case XxHash64(children, seed) =>
val firstUnSupportedInput = children.find(c => !supportedDataType(c.dataType))
```
It seems unlikely that all these expressions have the exact same supported types and it is also unlikely that we have tests today covering all the types that we claim to support.
## No data type checks
In some cases, we have no checks at all. For example, for `Sha2` we have no checks:
```scala
case Sha2(left, numBits) =>
```
Sha2 in Spark supports `StringType` and `BinaryType` input, but we only have tests for `StringType`, so there is a gap. Theoretically, we may produce incompatible results for `BinaryType` (we won't know until we add tests).
## Preparing for complex type support
Comet will soon support reading complex types (array, struct, map) from Parquet and Iceberg, so we must update the type checks to determine which expressions support which complex types. We cannot simply update `supportedDataType` and say that all expressions now also support all complex types.
### Describe the potential solution
_No response_
### Additional context
_No response_
--
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@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] [EPIC] Improve validation logic and testing for supported data types for expressions [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on issue #1345:
URL: https://github.com/apache/datafusion-comet/issues/1345#issuecomment-3825963593
This was addressed with the QueryPlanSerde refactor to use the expression serde framework
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] [EPIC] Improve validation logic and testing for supported data types for expressions [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove closed issue #1345: [EPIC] Improve validation logic and testing for supported data types for expressions
URL: https://github.com/apache/datafusion-comet/issues/1345
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org