Skip to content

test: remove "Comet (Scan)" cases from microbenchmarks#4258

Open
andygrove wants to merge 1 commit intoapache:mainfrom
andygrove:remove-scan-only-microbenchmarks
Open

test: remove "Comet (Scan)" cases from microbenchmarks#4258
andygrove wants to merge 1 commit intoapache:mainfrom
andygrove:remove-scan-only-microbenchmarks

Conversation

@andygrove
Copy link
Copy Markdown
Member

@andygrove andygrove commented May 7, 2026

Which issue does this PR close?

Closes #.

Rationale for this change

The "scan only" benchmark runs disabled spark.comet.exec.enabled, which actually disables scans now that native_datafusion is the default scan.

What changes are included in this PR?

Just run Spark vs Comet, where Comet is fully enabled.

How are these changes tested?

The "Comet (Scan)" case set COMET_ENABLED=true with COMET_EXEC_ENABLED=false,
intending to isolate scan performance. With spark.comet.scan.impl=auto (the
default), CometScanRule.nativeDataFusionScan refuses to install when exec is
disabled, so the case actually measured native_iceberg_compat scan plus
Spark ColumnarToRow. Comparing it against the other Comet case (which uses
native_datafusion plus CometNativeColumnarToRow) made the result a proxy for
scan-impl choice rather than the intended isolation.

Drop the "Comet (Scan)" cases everywhere and rename the combined case to
"Comet". Update doc comments accordingly.
Copy link
Copy Markdown
Contributor

@mbutrovich mbutrovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, thanks @andygrove!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants