Skip to content

Conversation

kazantsev-maksim
Copy link
Contributor

@kazantsev-maksim kazantsev-maksim commented Oct 2, 2025

Which issue does this PR close?

Part of: #2443

Rationale for this change

For the bitmap_count and read_side_padding functions, Spark uses a StaticInvoke wrapper. (https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/bitmapExpressions.scala#L85)

What changes are included in this PR?

  1. statics.scala - CometStaticInvoke wrapper has been added for the comet functions: bitmap_count, read_side_padding
  2. QueryPlanSerde.scala - removed unused matching cases
  3. CometBitmapExpressionSuite.scala - new UT

How are these changes tested?

A new UT has been added.

@kazantsev-maksim kazantsev-maksim marked this pull request as draft October 2, 2025 18:04
@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 56.25000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.74%. Comparing base (f09f8af) to head (728b7c9).
⚠️ Report is 581 commits behind head on main.

Files with missing lines Patch % Lines
...rc/main/scala/org/apache/comet/serde/statics.scala 50.00% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2518      +/-   ##
============================================
+ Coverage     56.12%   58.74%   +2.61%     
- Complexity      976     1452     +476     
============================================
  Files           119      148      +29     
  Lines         11743    13642    +1899     
  Branches       2251     2362     +111     
============================================
+ Hits           6591     8014    +1423     
- Misses         4012     4405     +393     
- Partials       1140     1223      +83     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kazantsev-maksim kazantsev-maksim changed the title bitmap_count draft impl Feat: bitmap_count impl and some refactoring Oct 4, 2025
@kazantsev-maksim kazantsev-maksim marked this pull request as ready for review October 4, 2025 16:28
Kazantsev Maksim added 2 commits October 4, 2025 20:31
@kazantsev-maksim kazantsev-maksim marked this pull request as draft October 7, 2025 16:54
@kazantsev-maksim kazantsev-maksim marked this pull request as ready for review October 7, 2025 17:08
@comphead
Copy link
Contributor

comphead commented Oct 9, 2025

Starting CI

@kazantsev-maksim
Copy link
Contributor Author

Thanks @comphead, but the test failures occurred due to DataFusion's lack of support for dictionary-encoded types.

Cause: org.apache.comet.CometNativeException: Error from DataFusion: bitmap_count expects Binary/BinaryView/FixedSizeBinary/LargeBinary as argument, got Dictionary(Int32, Binary).

@kazantsev-maksim
Copy link
Contributor Author

Created a task: apache/datafusion#18058

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.

3 participants