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/10/20 20:22:23 UTC
[PR] feat: Define function signatures in CometFuzz [WIP] [datafusion-comet]
andygrove opened a new pull request, #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Part of https://github.com/apache/datafusion-comet/issues/2611
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
The fuzz tester currently passes random inputs to functions without checking if they are the correct type. For example, it could try and pass a string to a numeric function. Although this can be a valid test (because Spark will add a cast to coerce the input type) it also means that many generated queries are not valid.
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
This PR is WIP but the goal is:
- Define signatures for functions
- Update query generator to only generate queries using valid input columns for the functions
## How are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
--
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: [PR] feat: Define function signatures in CometFuzz [WIP] [datafusion-comet]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3423750563
## [Codecov](https://app.codecov.io/gh/apache/datafusion-comet/pull/2614?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 11.01%. Comparing base ([`f09f8af`](https://app.codecov.io/gh/apache/datafusion-comet/commit/f09f8af64c6599255e116a376f4f008f2fd63b43?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`b087839`](https://app.codecov.io/gh/apache/datafusion-comet/commit/b0878390a4ae932d5d5534f9ca37ba3c7460459f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 625 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #2614 +/- ##
=============================================
- Coverage 56.12% 11.01% -45.12%
+ Complexity 976 232 -744
=============================================
Files 119 146 +27
Lines 11743 13736 +1993
Branches 2251 2335 +84
=============================================
- Hits 6591 1513 -5078
- Misses 4012 12046 +8034
+ Partials 1140 177 -963
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/datafusion-comet/pull/2614?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3428637615
> > > It really doesn't. As an example, if we add a new function that only exists in Spark 4.0 then run the fuzz test against an older version, the query will fail both with Spark and Comet, so that is a pass.
> >
> >
> > What if the signature changes in a new Spark release? Then it would start failing for Spark and Comet (and thus pass)?
> > I'm just trying to understand what the maintenance process is for future releases, and how to potentially document that.
>
> That is true. This is all very manual at the moment.
>
> I briefly looked into using Spark APIs to get the signature, but there are some challenges. We can look at classes to see if they extend `UnaryExpression` or `BinaryExpression`, but to determine the valid input data types we would need to create an instance of the class, which seems challenging.
@mbutrovich One option I would like to explore is to produce a summary report after running the queries, which would show how many successful queries ran for each expression and show error messages for any expressions that always failed
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3428496897
> > It really doesn't. As an example, if we add a new function that only exists in Spark 4.0 then run the fuzz test against an older version, the query will fail both with Spark and Comet, so that is a pass.
>
> What if the signature changes in a new Spark release? Then it would start failing for Spark and Comet (and thus pass)?
>
> I'm just trying to understand what the maintenance process is for future releases, and how to potentially document that.
That is true. This is all very manual at the moment.
I briefly looked into using Spark APIs to get the signature, but there are some challenges. We can look at classes to see if they extend `UnaryExpression` or `BinaryExpression`, but to determine the valid input data types we would need to create an instance of the class, which seems challenging.
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449452211
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
+ Function(name, Seq(FunctionSignature(inputs)))
+ }
+ private def createFunctions(name: String, signatures: Seq[FunctionSignature]): Function = {
+ Function(name, signatures)
+ }
+
+ private def createUnaryStringFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkStringType))
+ }
+
+ private def createUnaryNumericFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkNumericType))
+ }
+
+ // Math expressions (corresponds to mathExpressions in QueryPlanSerde)
val mathScalarFunc: Seq[Function] = Seq(
- Function("abs", 1),
- Function("acos", 1),
- Function("asin", 1),
- Function("atan", 1),
- Function("Atan2", 1),
- Function("Cos", 1),
- Function("Exp", 2),
- Function("Ln", 1),
- Function("Log10", 1),
- Function("Log2", 1),
- Function("Pow", 2),
- Function("Round", 1),
- Function("Signum", 1),
- Function("Sin", 1),
- Function("Sqrt", 1),
- Function("Tan", 1),
- Function("Ceil", 1),
- Function("Floor", 1),
- Function("bool_and", 1),
- Function("bool_or", 1),
- Function("bitwise_not", 1))
+ createUnaryNumericFunction("abs"),
+ createUnaryNumericFunction("acos"),
+ createUnaryNumericFunction("asin"),
+ createUnaryNumericFunction("atan"),
+ createFunctionWithInputs("atan2", Seq(SparkNumericType, SparkNumericType)),
+ createUnaryNumericFunction("cos"),
+ createUnaryNumericFunction("exp"),
+ createUnaryNumericFunction("expm1"),
+ createFunctionWithInputs("log", Seq(SparkNumericType, SparkNumericType)),
+ createUnaryNumericFunction("log10"),
+ createUnaryNumericFunction("log2"),
+ createFunctionWithInputs("pow", Seq(SparkNumericType, SparkNumericType)),
+ createFunctionWithInputs("remainder", Seq(SparkNumericType, SparkNumericType)),
+ createFunctions(
+ "round",
+ Seq(
+ FunctionSignature(Seq(SparkNumericType)),
+ FunctionSignature(Seq(SparkNumericType, SparkIntType)))),
+ createUnaryNumericFunction("signum"),
+ createUnaryNumericFunction("sin"),
+ createUnaryNumericFunction("sqrt"),
+ createUnaryNumericFunction("tan"),
+ createUnaryNumericFunction("ceil"),
+ createUnaryNumericFunction("floor"),
+ createFunctionWithInputs("unary_minus", Seq(SparkNumericType)))
+
+ // Hash expressions (corresponds to hashExpressions in QueryPlanSerde)
+ val hashScalarFunc: Seq[Function] = Seq(
+ createFunctionWithInputs("md5", Seq(SparkAnyType)),
+ createFunctionWithInputs("murmur3_hash", Seq(SparkAnyType)), // TODO can take multiple columns
+ createFunctionWithInputs("sha2", Seq(SparkAnyType, SparkIntType)))
+ // String expressions (corresponds to stringExpressions in QueryPlanSerde)
+ val stringScalarFunc: Seq[Function] = Seq(
+ createUnaryStringFunction("ascii"),
+ createUnaryStringFunction("bit_length"),
+ createUnaryStringFunction("chr"),
+ createFunctionWithInputs("concat_ws", Seq(SparkStringType, SparkStringType)),
Review Comment:
I added support for concat with two arguments. According to the [docs](https://spark.apache.org/docs/latest/api/python/reference/pyspark.sql/api/pyspark.sql.functions.concat.html):
```
The function works with strings, numeric, binary and compatible array columns.
```
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove merged PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449266503
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
+ Function(name, Seq(FunctionSignature(inputs)))
+ }
+ private def createFunctions(name: String, signatures: Seq[FunctionSignature]): Function = {
+ Function(name, signatures)
+ }
+
+ private def createUnaryStringFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkStringType))
+ }
+
+ private def createUnaryNumericFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkNumericType))
+ }
+
+ // Math expressions (corresponds to mathExpressions in QueryPlanSerde)
val mathScalarFunc: Seq[Function] = Seq(
Review Comment:
perhaps we can validate that expressions in `QueryPlanSerde` not covered by fuzzer? it would help us having a consistent functions set we fuzz testing especially if a person added a new function
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449419010
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/QueryRunner.scala:
##########
@@ -144,7 +144,11 @@ object QueryRunner {
a.length == b.length && a.zip(b).forall(x => same(x._1, x._2))
case (a: WrappedArray[_], b: WrappedArray[_]) =>
a.length == b.length && a.zip(b).forall(x => same(x._1, x._2))
+ case (a: Row, b: Row) =>
+ // struct support
+ format(a) == format(b)
Review Comment:
I updated this
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3428100011
How does this change with different Spark versions, or does it?
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3438208790
Thanks for the review @comphead. @mbutrovich could you take another look?
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449253228
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
Review Comment:
```suggestion
private def createFunctionWithInputParams(name: String, inputs: Seq[SparkType]): Function = {
```
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3428243389
> How does this change with different Spark versions, or does it?
It really doesn't. As an example, if we add a new function that only exists in Spark 4.0 then run the fuzz test against an older version, the query will fail both with Spark and Comet, so that is a pass.
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2452208360
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
+ Function(name, Seq(FunctionSignature(inputs)))
+ }
+ private def createFunctions(name: String, signatures: Seq[FunctionSignature]): Function = {
+ Function(name, signatures)
+ }
+
+ private def createUnaryStringFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkStringType))
+ }
+
+ private def createUnaryNumericFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkNumericType))
+ }
+
+ // Math expressions (corresponds to mathExpressions in QueryPlanSerde)
val mathScalarFunc: Seq[Function] = Seq(
Review Comment:
I filed https://github.com/apache/datafusion-comet/issues/2627 for finding a way to automate this
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449272476
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
+ Function(name, Seq(FunctionSignature(inputs)))
+ }
+ private def createFunctions(name: String, signatures: Seq[FunctionSignature]): Function = {
+ Function(name, signatures)
+ }
+
+ private def createUnaryStringFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkStringType))
+ }
+
+ private def createUnaryNumericFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkNumericType))
+ }
+
+ // Math expressions (corresponds to mathExpressions in QueryPlanSerde)
val mathScalarFunc: Seq[Function] = Seq(
- Function("abs", 1),
- Function("acos", 1),
- Function("asin", 1),
- Function("atan", 1),
- Function("Atan2", 1),
- Function("Cos", 1),
- Function("Exp", 2),
- Function("Ln", 1),
- Function("Log10", 1),
- Function("Log2", 1),
- Function("Pow", 2),
- Function("Round", 1),
- Function("Signum", 1),
- Function("Sin", 1),
- Function("Sqrt", 1),
- Function("Tan", 1),
- Function("Ceil", 1),
- Function("Floor", 1),
- Function("bool_and", 1),
- Function("bool_or", 1),
- Function("bitwise_not", 1))
+ createUnaryNumericFunction("abs"),
+ createUnaryNumericFunction("acos"),
+ createUnaryNumericFunction("asin"),
+ createUnaryNumericFunction("atan"),
+ createFunctionWithInputs("atan2", Seq(SparkNumericType, SparkNumericType)),
+ createUnaryNumericFunction("cos"),
+ createUnaryNumericFunction("exp"),
+ createUnaryNumericFunction("expm1"),
+ createFunctionWithInputs("log", Seq(SparkNumericType, SparkNumericType)),
+ createUnaryNumericFunction("log10"),
+ createUnaryNumericFunction("log2"),
+ createFunctionWithInputs("pow", Seq(SparkNumericType, SparkNumericType)),
+ createFunctionWithInputs("remainder", Seq(SparkNumericType, SparkNumericType)),
+ createFunctions(
+ "round",
+ Seq(
+ FunctionSignature(Seq(SparkNumericType)),
+ FunctionSignature(Seq(SparkNumericType, SparkIntType)))),
+ createUnaryNumericFunction("signum"),
+ createUnaryNumericFunction("sin"),
+ createUnaryNumericFunction("sqrt"),
+ createUnaryNumericFunction("tan"),
+ createUnaryNumericFunction("ceil"),
+ createUnaryNumericFunction("floor"),
+ createFunctionWithInputs("unary_minus", Seq(SparkNumericType)))
+
+ // Hash expressions (corresponds to hashExpressions in QueryPlanSerde)
+ val hashScalarFunc: Seq[Function] = Seq(
+ createFunctionWithInputs("md5", Seq(SparkAnyType)),
+ createFunctionWithInputs("murmur3_hash", Seq(SparkAnyType)), // TODO can take multiple columns
+ createFunctionWithInputs("sha2", Seq(SparkAnyType, SparkIntType)))
+ // String expressions (corresponds to stringExpressions in QueryPlanSerde)
+ val stringScalarFunc: Seq[Function] = Seq(
+ createUnaryStringFunction("ascii"),
+ createUnaryStringFunction("bit_length"),
+ createUnaryStringFunction("chr"),
+ createFunctionWithInputs("concat_ws", Seq(SparkStringType, SparkStringType)),
Review Comment:
we should be supporting `concat` with strings input in 50.3.0 #2604 so need to add it there.
Btw @andygrove `concat` supports string or arrays as input, looks like this design supports it?
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449272212
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
+ Function(name, Seq(FunctionSignature(inputs)))
+ }
+ private def createFunctions(name: String, signatures: Seq[FunctionSignature]): Function = {
+ Function(name, signatures)
+ }
+
+ private def createUnaryStringFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkStringType))
+ }
+
+ private def createUnaryNumericFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkNumericType))
+ }
+
+ // Math expressions (corresponds to mathExpressions in QueryPlanSerde)
val mathScalarFunc: Seq[Function] = Seq(
Review Comment:
That would be nice. The main challenge is that in the expr map in `QueryPlanSerde` we just have class names and no mapping to SQL function name.
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449449887
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/QueryRunner.scala:
##########
@@ -133,6 +130,9 @@ object QueryRunner {
}
private def same(l: Any, r: Any): Boolean = {
+ if (l == null || r == null) {
+ return l == null && r == null
+ }
(l, r) match {
case (a: Float, b: Float) if a.isInfinity => b.isInfinity
Review Comment:
added
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449254844
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
Review Comment:
inputs might be confused with input data
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3433407351
Moved to draft until https://github.com/apache/datafusion-comet/pull/2629 is merged
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449323300
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
+ Function(name, Seq(FunctionSignature(inputs)))
+ }
+ private def createFunctions(name: String, signatures: Seq[FunctionSignature]): Function = {
+ Function(name, signatures)
+ }
+
+ private def createUnaryStringFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkStringType))
+ }
+
+ private def createUnaryNumericFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkNumericType))
+ }
+
+ // Math expressions (corresponds to mathExpressions in QueryPlanSerde)
val mathScalarFunc: Seq[Function] = Seq(
Review Comment:
My current approach is to use AI to detect any expressions in `QueryPlanSerde` that are not covered in the fuzz test.
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3428826254
For comparison, should we delegate this to Spark itself for simplest cases? 🤔
```
sparkDf.count = = cometDf.count // to see duplicates or missing rows
sparkDf.except(comeDf).union(cometDF.except(sparkDf)) // column-level checks
```
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3429044738
> For comparison, should we delegate this to Spark itself for simplest cases? 🤔
>
> ```
> sparkDf.count = = cometDf.count // to see duplicates or missing rows
> sparkDf.except(comeDf).union(cometDF.except(sparkDf)) // column-level checks
> ```
Will this involve re-executing the queries?
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3432567391
@mbutrovich @comphead Thanks for the reviews so far. CometFuzz is still quite experimental/hacky, but this PR expands coverage of tested functions and reduces the number of invalid queries generated now that we have signatures, so it seems worth merging in my opinion. It would be better to automate the discovery of function signatures rather than hand-code them. I filed https://github.com/apache/datafusion-comet/issues/2627 to explore this.
Here are stats from a recent run of this version:
```
Total queries: 853; Invalid queries: 317; Comet failed: 3; Comet succeeded: 533
```
So far, this version of CometFuzz has found three correctness issues:
- https://github.com/apache/datafusion-comet/issues/2624
- https://github.com/apache/datafusion-comet/issues/2621
- https://github.com/apache/datafusion-comet/issues/2626
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3428312168
> It really doesn't. As an example, if we add a new function that only exists in Spark 4.0 then run the fuzz test against an older version, the query will fail both with Spark and Comet, so that is a pass.
What if the signature changes in a new Spark release? Then it would start failing for Spark and Comet (and thus pass)?
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449282053
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/QueryRunner.scala:
##########
@@ -144,7 +144,11 @@ object QueryRunner {
a.length == b.length && a.zip(b).forall(x => same(x._1, x._2))
case (a: WrappedArray[_], b: WrappedArray[_]) =>
a.length == b.length && a.zip(b).forall(x => same(x._1, x._2))
+ case (a: Row, b: Row) =>
+ // struct support
+ format(a) == format(b)
Review Comment:
Yes. This could probably be made more efficient.
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449286239
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/QueryRunner.scala:
##########
@@ -133,6 +130,9 @@ object QueryRunner {
}
private def same(l: Any, r: Any): Boolean = {
+ if (l == null || r == null) {
+ return l == null && r == null
+ }
(l, r) match {
case (a: Float, b: Float) if a.isInfinity => b.isInfinity
Review Comment:
should we also check `negInfinity` and `posInfinity`
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449275747
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
+ Function(name, Seq(FunctionSignature(inputs)))
+ }
+ private def createFunctions(name: String, signatures: Seq[FunctionSignature]): Function = {
+ Function(name, signatures)
+ }
+
+ private def createUnaryStringFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkStringType))
+ }
+
+ private def createUnaryNumericFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkNumericType))
+ }
+
+ // Math expressions (corresponds to mathExpressions in QueryPlanSerde)
val mathScalarFunc: Seq[Function] = Seq(
- Function("abs", 1),
- Function("acos", 1),
- Function("asin", 1),
- Function("atan", 1),
- Function("Atan2", 1),
- Function("Cos", 1),
- Function("Exp", 2),
- Function("Ln", 1),
- Function("Log10", 1),
- Function("Log2", 1),
- Function("Pow", 2),
- Function("Round", 1),
- Function("Signum", 1),
- Function("Sin", 1),
- Function("Sqrt", 1),
- Function("Tan", 1),
- Function("Ceil", 1),
- Function("Floor", 1),
- Function("bool_and", 1),
- Function("bool_or", 1),
- Function("bitwise_not", 1))
+ createUnaryNumericFunction("abs"),
+ createUnaryNumericFunction("acos"),
+ createUnaryNumericFunction("asin"),
+ createUnaryNumericFunction("atan"),
+ createFunctionWithInputs("atan2", Seq(SparkNumericType, SparkNumericType)),
+ createUnaryNumericFunction("cos"),
+ createUnaryNumericFunction("exp"),
+ createUnaryNumericFunction("expm1"),
+ createFunctionWithInputs("log", Seq(SparkNumericType, SparkNumericType)),
+ createUnaryNumericFunction("log10"),
+ createUnaryNumericFunction("log2"),
+ createFunctionWithInputs("pow", Seq(SparkNumericType, SparkNumericType)),
+ createFunctionWithInputs("remainder", Seq(SparkNumericType, SparkNumericType)),
+ createFunctions(
+ "round",
+ Seq(
+ FunctionSignature(Seq(SparkNumericType)),
+ FunctionSignature(Seq(SparkNumericType, SparkIntType)))),
+ createUnaryNumericFunction("signum"),
+ createUnaryNumericFunction("sin"),
+ createUnaryNumericFunction("sqrt"),
+ createUnaryNumericFunction("tan"),
+ createUnaryNumericFunction("ceil"),
+ createUnaryNumericFunction("floor"),
+ createFunctionWithInputs("unary_minus", Seq(SparkNumericType)))
+
+ // Hash expressions (corresponds to hashExpressions in QueryPlanSerde)
+ val hashScalarFunc: Seq[Function] = Seq(
+ createFunctionWithInputs("md5", Seq(SparkAnyType)),
+ createFunctionWithInputs("murmur3_hash", Seq(SparkAnyType)), // TODO can take multiple columns
+ createFunctionWithInputs("sha2", Seq(SparkAnyType, SparkIntType)))
+ // String expressions (corresponds to stringExpressions in QueryPlanSerde)
+ val stringScalarFunc: Seq[Function] = Seq(
+ createUnaryStringFunction("ascii"),
+ createUnaryStringFunction("bit_length"),
+ createUnaryStringFunction("chr"),
+ createFunctionWithInputs("concat_ws", Seq(SparkStringType, SparkStringType)),
Review Comment:
Yes, the framework supports it. In this case we could add two signatures to the function. One that takes two strings and one that takes two arrays. I did not implement support for variadic functions yet. I will file an issue for that.
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449281190
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
+ Function(name, Seq(FunctionSignature(inputs)))
+ }
+ private def createFunctions(name: String, signatures: Seq[FunctionSignature]): Function = {
+ Function(name, signatures)
+ }
+
+ private def createUnaryStringFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkStringType))
+ }
+
+ private def createUnaryNumericFunction(name: String): Function = {
+ createFunctionWithInputs(name, Seq(SparkNumericType))
+ }
+
+ // Math expressions (corresponds to mathExpressions in QueryPlanSerde)
val mathScalarFunc: Seq[Function] = Seq(
- Function("abs", 1),
- Function("acos", 1),
- Function("asin", 1),
- Function("atan", 1),
- Function("Atan2", 1),
- Function("Cos", 1),
- Function("Exp", 2),
- Function("Ln", 1),
- Function("Log10", 1),
- Function("Log2", 1),
- Function("Pow", 2),
- Function("Round", 1),
- Function("Signum", 1),
- Function("Sin", 1),
- Function("Sqrt", 1),
- Function("Tan", 1),
- Function("Ceil", 1),
- Function("Floor", 1),
- Function("bool_and", 1),
- Function("bool_or", 1),
- Function("bitwise_not", 1))
+ createUnaryNumericFunction("abs"),
+ createUnaryNumericFunction("acos"),
+ createUnaryNumericFunction("asin"),
+ createUnaryNumericFunction("atan"),
+ createFunctionWithInputs("atan2", Seq(SparkNumericType, SparkNumericType)),
+ createUnaryNumericFunction("cos"),
+ createUnaryNumericFunction("exp"),
+ createUnaryNumericFunction("expm1"),
+ createFunctionWithInputs("log", Seq(SparkNumericType, SparkNumericType)),
+ createUnaryNumericFunction("log10"),
+ createUnaryNumericFunction("log2"),
+ createFunctionWithInputs("pow", Seq(SparkNumericType, SparkNumericType)),
+ createFunctionWithInputs("remainder", Seq(SparkNumericType, SparkNumericType)),
+ createFunctions(
+ "round",
+ Seq(
+ FunctionSignature(Seq(SparkNumericType)),
+ FunctionSignature(Seq(SparkNumericType, SparkIntType)))),
+ createUnaryNumericFunction("signum"),
+ createUnaryNumericFunction("sin"),
+ createUnaryNumericFunction("sqrt"),
+ createUnaryNumericFunction("tan"),
+ createUnaryNumericFunction("ceil"),
+ createUnaryNumericFunction("floor"),
+ createFunctionWithInputs("unary_minus", Seq(SparkNumericType)))
+
+ // Hash expressions (corresponds to hashExpressions in QueryPlanSerde)
+ val hashScalarFunc: Seq[Function] = Seq(
+ createFunctionWithInputs("md5", Seq(SparkAnyType)),
+ createFunctionWithInputs("murmur3_hash", Seq(SparkAnyType)), // TODO can take multiple columns
+ createFunctionWithInputs("sha2", Seq(SparkAnyType, SparkIntType)))
+ // String expressions (corresponds to stringExpressions in QueryPlanSerde)
+ val stringScalarFunc: Seq[Function] = Seq(
+ createUnaryStringFunction("ascii"),
+ createUnaryStringFunction("bit_length"),
+ createUnaryStringFunction("chr"),
+ createFunctionWithInputs("concat_ws", Seq(SparkStringType, SparkStringType)),
Review Comment:
https://github.com/apache/datafusion-comet/issues/2617
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2449280803
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/QueryRunner.scala:
##########
@@ -144,7 +144,11 @@ object QueryRunner {
a.length == b.length && a.zip(b).forall(x => same(x._1, x._2))
case (a: WrappedArray[_], b: WrappedArray[_]) =>
a.length == b.length && a.zip(b).forall(x => same(x._1, x._2))
+ case (a: Row, b: Row) =>
+ // struct support
+ format(a) == format(b)
Review Comment:
I'm not sure what is compared here? is it text representation of structs?
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3429128283
> > For comparison, should we delegate this to Spark itself for simplest cases? 🤔
> > ```
> > sparkDf.count = = cometDf.count // to see duplicates or missing rows
> > sparkDf.except(comeDf).union(cometDF.except(sparkDf)) // column-level checks
> > ```
>
> Will this involve re-executing the queries?
Yes, both rows would be actions and trigger the dataframe evaluation. to avoid this we can cache/checkpoint dataframes, so it would be evaluated only once. Another option is to save both result df to disk and then read them for correctness checks, this option may also help in the future when Comet support the parquet writer.
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3432542263
> For comparison, should we delegate this to Spark itself for simplest cases? 🤔
>
> ```
> sparkDf.count = = cometDf.count // to see duplicates or missing rows
> sparkDf.except(comeDf).union(cometDF.except(sparkDf)) // column-level checks
> ```
I'm not sure how we can run `cometDF.except(sparkDf)` with `cometDF` running with Comet enabled and `sparkDf` running with Comet disabled
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3431489253
> I briefly looked into using Spark APIs to get the signature, but there are some challenges.
We may be able to obtain function signatures in these ways:
+ Get input types from [ExpectsInputTypes](https://github.com/apache/spark/blob/3109488d1f5afc9ef2a419c8972ddb0e9fff6610/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ExpectsInputTypes.scala#L43) trait
+ Parse function signatures from example sql, refer: [ExpressionsSchemaSuite.scala](https://github.com/apache/spark/blob/master/sql/core/src/test/scala/org/apache/spark/sql/ExpressionsSchemaSuite.scala)
+ Parse argument number from function desc, like `_FUNC_(str, charset)` in [org.apache.spark.sql.catalyst.expressions.Encode desc](https://github.com/apache/spark/blob/3109488d1f5afc9ef2a419c8972ddb0e9fff6610/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala#L3155)
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#issuecomment-3432511953
> * Get input types from [ExpectsInputTypes](https://github.com/apache/spark/blob/3109488d1f5afc9ef2a419c8972ddb0e9fff6610/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ExpectsInputTypes.scala#L43) trait
>
> * Parse function signatures from example sql, refer: [ExpressionsSchemaSuite.scala](https://github.com/apache/spark/blob/master/sql/core/src/test/scala/org/apache/spark/sql/ExpressionsSchemaSuite.scala)
>
> * Parse argument number from function desc, like `_FUNC_(str, charset)` in [org.apache.spark.sql.catalyst.expressions.Encode desc](https://github.com/apache/spark/blob/3109488d1f5afc9ef2a419c8972ddb0e9fff6610/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala#L3155)
Thanks. I filed an issue for this https://github.com/apache/datafusion-comet/issues/2627
--
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: [PR] feat: Define function signatures in CometFuzz [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #2614:
URL: https://github.com/apache/datafusion-comet/pull/2614#discussion_r2453379267
##########
fuzz-testing/src/main/scala/org/apache/comet/fuzz/Meta.scala:
##########
@@ -35,100 +61,261 @@ object Meta {
(DataTypes.createDecimalType(10, 2), 0.2),
(DataTypes.DateType, 0.2),
(DataTypes.TimestampType, 0.2),
- // TimestampNTZType only in Spark 3.4+
- // (DataTypes.TimestampNTZType, 0.2),
+ (DataTypes.TimestampNTZType, 0.2),
(DataTypes.StringType, 0.2),
(DataTypes.BinaryType, 0.1))
- val stringScalarFunc: Seq[Function] = Seq(
- Function("substring", 3),
- Function("coalesce", 1),
- Function("starts_with", 2),
- Function("ends_with", 2),
- Function("contains", 2),
- Function("ascii", 1),
- Function("bit_length", 1),
- Function("octet_length", 1),
- Function("upper", 1),
- Function("lower", 1),
- Function("chr", 1),
- Function("init_cap", 1),
- Function("trim", 1),
- Function("ltrim", 1),
- Function("rtrim", 1),
- Function("string_space", 1),
- Function("rpad", 2),
- Function("rpad", 3), // rpad can have 2 or 3 arguments
- Function("hex", 1),
- Function("unhex", 1),
- Function("xxhash64", 1),
- Function("sha1", 1),
- // Function("sha2", 1), -- needs a second argument for number of bits
- Function("substring", 3),
- Function("btrim", 1),
- Function("concat_ws", 2),
- Function("repeat", 2),
- Function("length", 1),
- Function("reverse", 1),
- Function("instr", 2),
- Function("replace", 2),
- Function("translate", 2))
-
- val dateScalarFunc: Seq[Function] =
- Seq(Function("year", 1), Function("hour", 1), Function("minute", 1), Function("second", 1))
+ private def createFunctionWithInputs(name: String, inputs: Seq[SparkType]): Function = {
Review Comment:
I renamed to `createFunctionWithInputTypes`
--
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