Skip to content

fix: link accumulator-related IgnoreCometNativeScan tests to #3867#3830

Open
andygrove wants to merge 1 commit intomainfrom
fix/spark-343-filter-pushdown-tag
Open

fix: link accumulator-related IgnoreCometNativeScan tests to #3867#3830
andygrove wants to merge 1 commit intomainfrom
fix/spark-343-filter-pushdown-tag

Conversation

@andygrove
Copy link
Copy Markdown
Member

@andygrove andygrove commented Mar 28, 2026

Which issue does this PR close?

Closes #3867.

Rationale for this change

Several Spark SQL tests in the Parquet filter suite are ignored under IgnoreCometNativeScan because native scans cannot propagate JVM-side Spark accumulators (e.g. NumRowGroupsAcc). This does not affect query correctness or user-visible behavior — filter pushdown works correctly in native scans. The limitation is that the test mechanism (JVM-side accumulators) does not work across the JNI boundary.

The ignore annotations previously used inconsistent reason strings ("cannot be pushed down", "Native scans do not support the tested accumulator") and did not link to a tracking issue.

What changes are included in this PR?

Updated all accumulator-related IgnoreCometNativeScan annotations across all three Spark version diffs (3.4.3, 3.5.8, 4.0.1) to reference the new tracking issue #3867. The three affected tests in each diff are:

  • filter pushdown - StringPredicate
  • Filters should be pushed down for vectorized Parquet reader at row group level
  • SPARK-34562: Bloom filter push down

How are these changes tested?

These are test annotation changes only. The tests themselves are unchanged and remain correctly ignored when running with native scans.

@andygrove andygrove changed the title fix: use IgnoreCometNativeScan for StringPredicate test in 3.4.3 diff minor: use IgnoreCometNativeScan for StringPredicate test in 3.4.3 diff Mar 28, 2026
- test("filter pushdown - StringPredicate") {
+ test("filter pushdown - StringPredicate",
+ IgnoreCometNativeDataFusion("cannot be pushed down")) {
+ IgnoreCometNativeScan("cannot be pushed down")) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would you mind putting a link to the issue please?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is no issue for this one. It isn't due to a bug in Comet, but a limitation of how the Spark test is written.

Copy link
Copy Markdown
Contributor

@kazuyukitanimura kazuyukitanimura Mar 30, 2026

Choose a reason for hiding this comment

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

Thanks.

Is #3321 the tracking issue?

The test fails because the native scan doesn't propagate the NumRowGroupsAcc Spark accumulator So sounds like the gap is still there even it is not a bug? I was thinking we can include the link like IgnoreCometNativeScan("https://github.com/apache/datafusion-comet/issues/3321") so that we can revisit, just like other skipped tests

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I created a new specific issue and updated all relevant tests to link to the new issue #3867

@andygrove andygrove force-pushed the fix/spark-343-filter-pushdown-tag branch from 209885c to d8948a6 Compare April 1, 2026 10:55
@andygrove andygrove changed the title minor: use IgnoreCometNativeScan for StringPredicate test in 3.4.3 diff fix: link accumulator-related IgnoreCometNativeScan tests to #3867 Apr 1, 2026
Update all tests ignored because native scans cannot propagate
JVM-side Spark accumulators to reference the new tracking issue
across all three Spark version diffs (3.4.3, 3.5.8, 4.0.1).

Affected tests:
- filter pushdown - StringPredicate
- Filters should be pushed down for vectorized Parquet reader at row group level
- SPARK-34562: Bloom filter push down
@andygrove andygrove force-pushed the fix/spark-343-filter-pushdown-tag branch from d8948a6 to c0be213 Compare April 1, 2026 11:00
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.

Native scans cannot propagate JVM-side Spark accumulators

2 participants