fix: Native_datafusion reports correct files and bytes scanned by 0lai0 · Pull Request #3798 · apache/datafusion-comet

0lai0 · 2026-03-26T09:18:32Z

Which issue does this PR close?

Rationale for this change

In CometScanExec, calling getFilePartitions() unconditionally executes sendDriverMetrics(). Because getFilePartitions() can be evaluated multiple times during planning (e.g., converting to CometNativeScanExec) and execution (e.g., fetching partitions), the SQLMetric accumulators like numFiles and filesSize were being duplicated. This led to incorrect double-counted values rendering in the Spark UI.

What changes are included in this PR?

Replaced metrics(...).add() with metrics(...).set() in CometScanExec to ensure idempotency when reporting metrics.
Wrapped the driver metric updates and Spark listener event dispatching inside a lazy val. This prevents both double-counting during Catalyst transformations (makeCopy) and sending redundant UI events.

How are these changes tested?

Added a dedicated end-to-end unit test in CometExecSuite.
The test writes a dummy Parquet dataset, sequentially triggers multiple UI actions (count and collect) to force severe plan evaluations, and strictly asserts that numFiles is exactly 2 without any duplication.

andygrove

LGTM. Thanks @0lai0.

mbutrovich

So basically it's an artifact of wrapping CometNativeScan in CometScan, which we hopefully won't do in the future anyway.

Thanks for the fix in the meantime, @0lai0!

comphead

Thanks @0lai0 I'll quickly check it out today

comphead

Somehow on UI I can now see 0

number of files read: 0
size of files read: 0.0 B

comphead · 2026-03-27T00:11:00Z

spark/src/test/scala/org/apache/comet/exec/CometExecSuite.scala

+      spark.range(100).repartition(2).write.mode("overwrite").parquet(path)
+
+      withSQLConf(
+        CometConf.COMET_ENABLED.key -> "true",


Please include --conf spark.comet.scan.impl=native_datafusion

Thanks @comphead for review. I added this to the latest commit.

0lai0 · 2026-03-27T07:17:07Z

Thank you all for the feedback. I’ll investigate this matter and fix it.

mbutrovich · 2026-03-30T15:59:40Z

Marking this as draft so we don't accidentally merge it, feel free to flip it back when it's ready for another look. Thanks @0lai0!

0lai0 · 2026-03-31T03:04:31Z

Thanks @mbutrovich . I'm still investigating the issue and trying to reproduce the scenario, but I haven't identified the root cause yet.

0lai0 · 2026-04-02T09:05:42Z

Thank you all for the review.
I've updated the test to strictly use spark.comet.scan.impl=native_datafusion as requested.

After checking further, I've simplified the fix by changing metrics.add to metrics.set. This ensures idempotency: if Catalyst evaluates the metric multiple times, it updates to the same fixed value rather than accumulating (which would cause double-counting) or resetting to zero.

0lai0 · 2026-04-02T09:11:10Z

This is the show on UI.

spark.range(10000000).repartition(20).write.parquet(location) 
spark.read.parquet("location").show(false)

But I'm not sure whether it is the correct snapshot.

comphead · 2026-04-02T15:35:53Z

Checking, btw output_rows looks already also fixed in #3842

testing num Files

comphead · 2026-04-02T15:43:04Z

spark/src/test/scala/org/apache/comet/exec/CometExecSuite.scala

  }

+  test("Native_datafusion reports correct files and bytes scanned") {
+    withTempDir { dir =>


Suggested change

withTempDir { dir =>

val inputFiles = 2

withTempDir { dir =>

comphead · 2026-04-02T15:43:18Z

spark/src/test/scala/org/apache/comet/exec/CometExecSuite.scala

+  test("Native_datafusion reports correct files and bytes scanned") {
+    withTempDir { dir =>
+      val path = new java.io.File(dir, "test_metrics").getAbsolutePath
+      spark.range(100).repartition(2).write.mode("overwrite").parquet(path)


Suggested change

spark.range(100).repartition(2).write.mode("overwrite").parquet(path)

spark.range(100).repartition(inputFiles).write.mode("overwrite").parquet(path)

comphead · 2026-04-02T15:43:31Z

spark/src/test/scala/org/apache/comet/exec/CometExecSuite.scala

+
+        val numFiles = scanNode.metrics("numFiles").value
+        assert(
+          numFiles == 2,


Suggested change

numFiles == 2,

numFiles == inputFiles,

comphead · 2026-04-02T15:43:45Z

spark/src/test/scala/org/apache/comet/exec/CometExecSuite.scala

+        val numFiles = scanNode.metrics("numFiles").value
+        assert(
+          numFiles == 2,
+          s"Expected exactly 2 files to be scanned, but got metrics reporting $numFiles")


Suggested change

s"Expected exactly 2 files to be scanned, but got metrics reporting $numFiles")

s"Expected exactly $inputFiles files to be scanned, but got metrics reporting $numFiles")

comphead

Thanks @0lai0 I can see correct numbers now, please polish a test a little bit and this PR is good to go

comphead · 2026-04-02T19:58:56Z

I went ahead with the merge to test it sooner

0lai0 · 2026-04-03T07:10:07Z

Hi @comphead , apologies for the late reply! Thanks for taking care of the merge to test it sooner. I'll open a quick follow-up PR shortly to polish the test as you suggested.

…e#3798)

Native_datafusion reports correct files and bytes scanned

573872a

andygrove approved these changes Mar 26, 2026

View reviewed changes

andygrove requested a review from comphead March 26, 2026 13:07

mbutrovich reviewed Mar 26, 2026

View reviewed changes

comphead reviewed Mar 26, 2026

View reviewed changes

comphead reviewed Mar 27, 2026

View reviewed changes

mbutrovich marked this pull request as draft March 30, 2026 16:00

remove redundant lazy val and enable native scan

c94b22a

0lai0 marked this pull request as ready for review April 2, 2026 09:05

0lai0 requested a review from comphead April 2, 2026 09:18

comphead reviewed Apr 2, 2026

View reviewed changes

comphead merged commit 423b572 into apache:main Apr 2, 2026
158 checks passed

0lai0 mentioned this pull request Apr 3, 2026

fix: parameterize file count in Native_datafusion metrics test #3896

Merged

vaibhawvipul pushed a commit to vaibhawvipul/datafusion-comet that referenced this pull request Apr 4, 2026

fix: Native_datafusion reports correct files and bytes scanned (apach…

cefa127

…e#3798)

	spark.range(100).repartition(2).write.mode("overwrite").parquet(path)
	spark.range(100).repartition(inputFiles).write.mode("overwrite").parquet(path)

	s"Expected exactly 2 files to be scanned, but got metrics reporting $numFiles")
	s"Expected exactly $inputFiles files to be scanned, but got metrics reporting $numFiles")

Conversation

0lai0 commented Mar 26, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

How are these changes tested?

Uh oh!

andygrove left a comment

Choose a reason for hiding this comment

Uh oh!

mbutrovich left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

comphead left a comment

Choose a reason for hiding this comment

Uh oh!

comphead left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

comphead Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

0lai0 Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

0lai0 commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbutrovich commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0lai0 commented Mar 31, 2026

Uh oh!

0lai0 commented Apr 2, 2026

Uh oh!

0lai0 commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

comphead commented Apr 2, 2026

Uh oh!

comphead Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

comphead Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

comphead Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

comphead Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

comphead left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

comphead commented Apr 2, 2026

Uh oh!

0lai0 commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mbutrovich left a comment •

edited

Loading

comphead left a comment •

edited

Loading

0lai0 commented Mar 27, 2026 •

edited

Loading

mbutrovich commented Mar 30, 2026 •

edited

Loading

0lai0 commented Apr 2, 2026 •

edited

Loading