Skip to content

Conversation

@comphead
Copy link
Contributor

Which issue does this PR close?

Closes #2971 .

Rationale for this change

Comet executed the write twice when AQE enabled, the write should be done only once. The reason was the AQE runs its set of rules and applies modifications based on original plan and looks for WriteFilesExec or DataWriteCommandExec. Those command were replaced by Comet and AQE inserts another write layer on top of Comet layer

What changes are included in this PR?

How are these changes tested?

assert(
hasNativeWrite,
s"Expected CometNativeWriteExec in the plan, but got:\n${executedPlan.treeString}")
nativeWriteCount == 1,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

check the count is just 1

@comphead comphead requested a review from andygrove December 24, 2025 00:26
@codecov-commenter
Copy link

codecov-commenter commented Dec 24, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 59.59%. Comparing base (f09f8af) to head (1f0170b).
⚠️ Report is 800 commits behind head on main.

Files with missing lines Patch % Lines
...n/scala/org/apache/comet/rules/CometExecRule.scala 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2982      +/-   ##
============================================
+ Coverage     56.12%   59.59%   +3.46%     
- Complexity      976     1366     +390     
============================================
  Files           119      167      +48     
  Lines         11743    15495    +3752     
  Branches       2251     2569     +318     
============================================
+ Hits           6591     9234    +2643     
- Misses         4012     4962     +950     
- Partials       1140     1299     +159     

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

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @comphead

@comphead comphead merged commit 0ae6515 into apache:main Dec 24, 2025
117 checks passed
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.

Comet writer executes twice

3 participants