Skip to content

Conversation

@andygrove
Copy link
Member

@andygrove andygrove commented Nov 12, 2025

Which issue does this PR close?

Part of #2757

Rationale for this change

This is a small step in a larger refactoring.

operator2Proto is only called from CometExecRule, so this PR moves it to that class and makes it a private method.

This moves all the code for determining whether native operators are enabled and supported into a single file.

What changes are included in this PR?

There are no functional changes in this PR. It just moves some code around.

The next PR after this (#2768) is a little more interesting.

How are these changes tested?

@codecov-commenter
Copy link

codecov-commenter commented Nov 12, 2025

Codecov Report

❌ Patch coverage is 70.07874% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.84%. Comparing base (f09f8af) to head (0d3d72c).
⚠️ Report is 692 commits behind head on main.

Files with missing lines Patch % Lines
...n/scala/org/apache/comet/rules/CometExecRule.scala 70.07% 22 Missing and 16 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2767      +/-   ##
============================================
+ Coverage     56.12%   58.84%   +2.71%     
- Complexity      976     1501     +525     
============================================
  Files           119      164      +45     
  Lines         11743    14014    +2271     
  Branches       2251     2371     +120     
============================================
+ Hits           6591     8246    +1655     
- Misses         4012     4567     +555     
- Partials       1140     1201      +61     

☔ 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.

Copy link
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.

Looks like an improvement to me, thanks @andygrove!

@mbutrovich mbutrovich merged commit 5fa091e into apache:main Nov 13, 2025
118 checks passed
mbutrovich added a commit to mbutrovich/datafusion-comet that referenced this pull request Nov 13, 2025
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