andygrove opened a new issue, #5372:
URL: https://github.com/apache/datafusion-comet/issues/5372
## Describe the bug
Nine rows across two expression benchmark suites are labelled `Comet` but
are measuring Spark. In each case the plan falls back to a JVM `Project`
sitting on top of `CometColumnarToRow`, so the expression itself never runs
natively.
`runExpressionBenchmark` has always checked for this, but it reported the
result with `println`, so the warning went to the console while the results
table went to the `.txt`. Anyone reading the table, or a copy of it pasted into
a PR, sees a `Comet` row with no indication that it ran on Spark. #5371 routes
that warning into the results file, which is how these surfaced.
## Affected rows
`CometStringExpressionBenchmark` (1 row):
```
*(1) Project [translate(c1#12, 123456, aBcDeF) AS translate(c1, 123456,
aBcDeF)#23815]
+- *(1) CometColumnarToRow
+- CometNativeScan parquet [c1#12] ...
```
`CometCastNumericToNumericBenchmark` (8 rows), all casts from `ShortType`:
```
Project [cast(c_short#19 as int) AS c_short#528]
Project [cast(c_short#19 as bigint) AS c_short#831L]
Project [cast(c_short#19 as tinyint) AS c_short#3765]
Project [cast(c_short#19 as float) AS c_short#5321]
Project [try_cast(c_short#19 as int) AS c_short#1904]
Project [try_cast(c_short#19 as bigint) AS c_short#2218L]
Project [try_cast(c_short#19 as tinyint) AS c_short#4308]
Project [try_cast(c_short#19 as float) AS c_short#6690]
```
The short cases are the surprising ones: Comet supports short-to-int and
short-to-long casts, and the equivalent casts from other numeric types in the
same suite do run natively. Something specific to `ShortType` in this suite is
preventing the projection from being replaced. Worth understanding before
assuming the benchmark is simply exercising an unsupported path.
## Steps to reproduce
With #5371 applied:
```sh
SPARK_GENERATE_BENCHMARK_FILES=1 make
benchmark-org.apache.spark.sql.benchmark.CometCastNumericToNumericBenchmark
SPARK_GENERATE_BENCHMARK_FILES=1 make
benchmark-org.apache.spark.sql.benchmark.CometStringExpressionBenchmark
```
Then grep the generated files under `spark/benchmarks/` for `First non-Comet
operator`.
Without #5371, the same warnings appear on stdout during the run.
## Expected behavior
Either the expressions run natively, or the benchmark is adjusted so the row
is not presented as a Comet measurement.
## Additional context
Found while working on #5363. The fallback is the bug here; the reporting
gap that hid it is fixed separately in #5371.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
